lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch v8 03/10] task isolation: sync vmstats on return to userspace
On Fri, Jan 21, 2022 at 01:06:10PM +0100, Frederic Weisbecker wrote:
> 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.

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.

\
 
 \ /
  Last update: 2022-01-27 17:48    [W:0.054 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site