Messages in this thread | | | Date | Tue, 17 Nov 2015 12:21:49 +0100 | From | Peter Zijlstra <> | Subject | Re: [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.
| |