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

Oscar Shiang
2022-03-16T02:42:18Z

On Mar 15, 2022, at 11:31 PM, Marcelo Tosatti <mtosatti@redhat.com> 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@redhat.com>
>
> ---
>
> 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