lkml.org 
[lkml]   [2015]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 3/3] sched: optimize migration by forcing rmb() and updating to be called once
On Tue, Nov 17, 2015 at 09:44:16AM +0900, Byungchul Park wrote:

> > So currently, set_task_cpu() is serialized by:
> >
> > - p->pi_lock; on wakeup
> > - rq->lock; otherwise
> >
> > (see the #ifdef CONFIG_LOCKDEP comment in set_task_cpu())
>
> I already read the comment.. Then do you mean the comment above
> migrate_task_rq_fair() is wrong and should be fixed?

Looks that way, I'm not sure we always hold pi_lock there. But I'm low
on sleep, so I could have overlooked something.

See for example move_queued_task(), we call set_task_cpu() with rq->lock
held, but no pi_lock.

> I thought the comment above migrate_task_rq_fair() is correct rather
> than CONFIG_LOCKDEP comment in set_task_cpu(), when I read it. I think
> these two comments are conflict each other a little bit, so one of
> those should be fixed.

Agreed.

> * the comment above migrate_task_rq_fair() describes it like,
> Caller SHOULD HOLD (&p->pi_lock)
>
> * the CONFIG_LOCKDEP comment in set_task_cpu() describes it like,
> Caller SHOULD HOLD (&p->pi_lock || &rq->lock)

Indeed.

> >
> > This means that sched_class::migrate_task() cannot indeed rely on
> > rq->lock for full serialization, however it still means that
> > task_rq_lock() will fully serialize against the thing.
>
> Yes I also think this is true.
>
> >
> > By changing this, it no longer will.
>
> ???

I meant, if you call __set_task_cpu() before
sched_class::migrate_task_rq(), in that case task_rq_lock() will no
longer fully serialize against set_task_cpu().

Because once you've called __set_task_cpu(), task_rq_lock() will acquire
the _other_ rq->lock. And we cannot rely on our rq->lock to serialize
things.


\
 
 \ /
  Last update: 2015-11-17 12:41    [W:1.015 / U:0.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site