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

[Frederic Weisbecker <frederic@kernel.org>]

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.