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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
