Messages in this thread | | | Date | Thu, 27 Jan 2022 19:01:51 +0100 | From | Frederic Weisbecker <> | Subject | Re: [patch v8 03/10] task isolation: sync vmstats on return to userspace |
| |
On Thu, Jan 27, 2022 at 01:47:07PM -0300, Marcelo Tosatti wrote: > > > @@ -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.
You're unconditionally adding overhead to the syscall fastpath when it would be so easy to set a TIF_FLAG as long as (current->task_isol_info->quiesce_mask != 0). vmstat_dirty can be checked afterward.
I suspect your patchset will sell much better if you join the common slowpath single condition in EXIT_TO_USER_MODE_WORK.
Thanks.
| |