Re: [patch v8 03/10] task isolation: sync vmstats on return to userspace

[Frederic Weisbecker <frederic@kernel.org>]

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.