Re: [PATCH] Synchronize task mm counters on context switch

[Minchan Kim <minchan@kernel.org>]

On Thu, Feb 22, 2018 at 10:40:09AM +0100, Peter Zijlstra wrote:
> On Thu, Feb 22, 2018 at 11:06:33AM +0900, Minchan Kim wrote:
> > On Wed, Feb 21, 2018 at 04:23:43PM -0800, Daniel Colascione wrote:
> > >  kernel/sched/core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index a7bf32aabfda..7f197a7698ee 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -3429,6 +3429,9 @@ asmlinkage __visible void __sched schedule(void)
> > >         struct task_struct *tsk = current;
> > >
> > >         sched_submit_work(tsk);
> > > +       if (tsk->mm)
> > > +               sync_mm_rss(tsk->mm);
> > > +
> > >         do {
> > >                 preempt_disable();
> > >                 __schedule(false);
> > >
> 
> Obviously I completely hate that; and you really _should_ have Cc'ed me
> earlier ;-)
> 
> That it still well over 100 cycles in the case when all counters did
> change. Far _far_ more if the mm counters are contended (up to 150 times
> more is quite possible).
> 
> > > > > Ping? Is this approach just a bad idea? We could instead just manually sync
> > > > > all mm-attached tasks at counter-retrieval time.
> > > >
> > > > IMHO, yes, it should be done when user want to see which would be really
> > > > cold path while this shecule function is hot.
> > > >
> > > 
> > > The problem with doing it that way is that we need to look at each task
> > > attached to a particular mm. AFAIK (and please tell me if I'm wrong), the
> > > only way to do that is to iterate over all processes, and for each process
> > > attached to the mm we want, iterate over all its tasks (since each one has
> > > to have the same mm, I think). Does that sound right?
> 
> You could just iterate the thread group and call it a day. Yes strictly
> speaking its possible to have mm's shared outside the thread group,
> practically that 'never' happens.
> 
> CLONE_VM without CLONE_THREAD just isn't a popular thing afaik.

That was a part I was not sure. ;-)

> 
> So while its not perfect, it might well be good enough.

If it's acceptable for everybody, yub, it's surely preferable, which sync
stats through the thread group at counter-retrieval time.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>