Re: [patch v12 09/13] task isolation: add preempt notifier to sync per-CPU vmstat dirty info to thread info

[Oscar Shiang]

On Mar 15, 2022, at 11:31 PM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
> If a thread has task isolation activated, is preempted by thread B,
> which marks vmstat information dirty, and is preempted back in,
> one might return to userspace with vmstat dirty information on the
> CPU in question.
> 
> To address this problem, add a preempt notifier that transfers vmstat dirty
> information to TIF_TASK_ISOL thread flag.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
> 
> ---
> 
> v12:
>  - switch from raw_cpu_read to __this_cpu_read (Frederic)
> 
> ---
>  include/linux/task_isolation.h |    2 ++
>  include/linux/vmstat.h         |    6 ++++++
>  kernel/task_isolation.c        |   23 +++++++++++++++++++++++
>  mm/vmstat.c                    |    7 +++++++
>  4 files changed, 38 insertions(+)
> 
> Index: linux-2.6/kernel/task_isolation.c
> ===================================================================
> --- linux-2.6.orig/kernel/task_isolation.c
> +++ linux-2.6/kernel/task_isolation.c
> @@ -19,6 +19,7 @@
>  #include <linux/sched/task.h>
>  #include <linux/mm.h>
>  #include <linux/vmstat.h>
> +#include <linux/preempt.h>
>  #include <linux/task_isolation.h>
> 
>  void __task_isol_exit(struct task_struct *tsk)
> @@ -30,6 +31,9 @@ void __task_isol_exit(struct task_struct
>  		return;
> 
>  	static_key_slow_dec(&vmstat_sync_enabled);
> +
> +	preempt_notifier_unregister(&i->preempt_notifier);
> +	preempt_notifier_dec();
>  }
> 
>  void __task_isol_free(struct task_struct *tsk)
> @@ -40,6 +44,21 @@ void __task_isol_free(struct task_struct
>  	tsk->task_isol_info = NULL;
>  }
> 
> +static void task_isol_sched_in(struct preempt_notifier *pn, int cpu)
> +{
> +	vmstat_dirty_to_thread_flag();
> +}
> +
> +static void task_isol_sched_out(struct preempt_notifier *pn,
> +				struct task_struct *next)
> +{
> +}
> +
> +static __read_mostly struct preempt_ops task_isol_preempt_ops = {
> +	.sched_in	= task_isol_sched_in,
> +	.sched_out	= task_isol_sched_out,
> +};
> +
>  static struct task_isol_info *task_isol_alloc_context(void)
>  {
>  	struct task_isol_info *info;
> @@ -48,6 +67,10 @@ static struct task_isol_info *task_isol_
>  	if (unlikely(!info))
>  		return ERR_PTR(-ENOMEM);
> 
> +	preempt_notifier_inc();
> +	preempt_notifier_init(&info->preempt_notifier, &task_isol_preempt_ops);
> +	preempt_notifier_register(&info->preempt_notifier);
> +
>  	preempt_disable();
>  	init_sync_vmstat();
>  	preempt_enable();
> 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
> @@ -17,6 +17,8 @@ struct task_isol_info {
>  	u64 oneshot_mask;
> 
>  	u8 inherit_mask;
> +
> +	struct preempt_notifier preempt_notifier;

Since preempt_notifier is visible only when CONFIG_KVM is enabled,
I think we can add KVM to the dependencies of CONFIG_TASK_ISOLATION.

Or we could encounter missing definition error while building without
CONFIG_KVM.

Thanks,
Oscar