Messages in this thread | | | Date | Thu, 27 Jan 2022 13:47:07 -0300 | From | Marcelo Tosatti <> | Subject | Re: [patch v8 03/10] task isolation: sync vmstats on return to userspace |
| |
On Fri, Jan 21, 2022 at 01:06:10PM +0100, Frederic Weisbecker wrote: > On Wed, Dec 08, 2021 at 01:09:09PM -0300, Marcelo Tosatti wrote: > > Index: linux-2.6/include/linux/task_isolation.h > > =================================================================== > > --- linux-2.6.orig/include/linux/task_isolation.h > > +++ linux-2.6/include/linux/task_isolation.h > > @@ -40,8 +40,19 @@ int prctl_task_isolation_activate_set(un > > > > int __copy_task_isolation(struct task_struct *tsk); > > > > +void isolation_exit_to_user_mode(void); > > + > > +static inline int task_isol_has_work(void) > > +{ > > + return 0; > > +} > > + > > #else > > > > +static void isolation_exit_to_user_mode(void) > > +{ > > +} > > + > > static inline void tsk_isol_free(struct task_struct *tsk) > > { > > } > > @@ -86,6 +97,11 @@ static inline int prctl_task_isolation_a > > return -EOPNOTSUPP; > > } > > > > +static inline int task_isol_has_work(void) > > +{ > > + return 0; > > +} > > + > > It would be nice to have a coherent greppable task_isol_*() namespace instead > of random scattered tsk_*(), isolation_*() stuff... > > task_isol_exit_to_user_mode() > task_isol_free() > task_isol_copy_process() > task_isol_had_work() > ... > > > @@ -149,13 +150,14 @@ static void handle_signal_work(struct pt > > } > > > > static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, > > - unsigned long ti_work) > > + unsigned long ti_work, > > + unsigned long tsk_isol_work) > > { > > /* > > * Before returning to user space ensure that all pending work > > * items have been completed. > > */ > > - while (ti_work & EXIT_TO_USER_MODE_WORK) { > > + while ((ti_work & EXIT_TO_USER_MODE_WORK) || tsk_isol_work) { > > So there is a dependency on CONFIG_GENERIC_ENTRY. Then you need to split that > from CONFIG_CPU_ISOLATION: > > config TASK_ISOLATION > bool "Task isolation prctl()" > depends on GENERIC_ENTRY > help "...." > > > > > local_irq_enable_exit_to_user(ti_work); > > > > @@ -177,6 +179,9 @@ static unsigned long exit_to_user_mode_l > > /* Architecture specific TIF work */ > > arch_exit_to_user_mode_work(regs, ti_work); > > > > + if (tsk_isol_work) > > + isolation_exit_to_user_mode(); > > + > > /* > > * Disable interrupts and reevaluate the work flags as they > > * might have changed while interrupts and preemption was > > @@ -188,6 +193,7 @@ static unsigned long exit_to_user_mode_l > > tick_nohz_user_enter_prepare(); > > > > ti_work = READ_ONCE(current_thread_info()->flags); > > + tsk_isol_work = task_isol_has_work(); > > Shouldn't it be a TIF_FLAG part of EXIT_TO_USER_MODE_WORK instead? > > Thanks.
static inline int task_isol_has_work(void) { int cpu, ret; struct isol_info *i;
if (likely(current->task_isol_info == NULL)) return 0;
i = current->task_isol_info; if (i->active_mask != ISOL_F_QUIESCE) return 0;
if (!(i->quiesce_mask & ISOL_F_QUIESCE_VMSTATS)) return 0;
cpu = get_cpu(); ret = per_cpu(vmstat_dirty, cpu); put_cpu();
return ret; }
Well, whether its necessary to call task_isol_exit_to_user_mode depends on the state of the enabled/disabled masks _and_ vmstat dirty bit information.
It seems awkward, to me, to condense all that information in a single bit.
Addressed all other comments, thanks.
| |