Messages in this thread | | | Subject | Re: lockdep-related warning in kernel/sched/deadline.c::find_lock_later_rq() | From | Luca Abeni <> | Date | Thu, 22 Oct 2015 09:06:31 +0200 |
| |
Hi,
On 10/22/2015 07:35 AM, Wanpeng Li wrote: [...] >> Now, if I understand correctly the issue is that dl_task_timer() does: >> rq = task_rq_lock(p, &flags); >> [...] >> if (has_pushable_dl_tasks(rq)) >> push_dl_task(rq); >> with task_rq_lock() that pins rq->lock and push_tl_task() that invokes >> find_lock_later_rq() that unlocks rq->lock() while it is pinned. >> >> I am not sure about how to fix this issue: as a first try, I did >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c >> index 142df26..5b1ba95 100644 >> --- a/kernel/sched/deadline.c >> +++ b/kernel/sched/deadline.c >> @@ -668,8 +668,11 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer) >> * Queueing this task back might have overloaded rq, check if we need >> * to kick someone away. >> */ >> - if (has_pushable_dl_tasks(rq)) >> + if (has_pushable_dl_tasks(rq)) { >> + lockdep_unpin_lock(&rq->lock); >> push_dl_task(rq); >> + lockdep_pin_lock(&rq->lock); >> + } >> #endif >> >> unlock: >> >> This removes the warning, but I am not sure if it is the correct fix (is it >> valid to unpin rq->lock, here?). >> >> If someone can confirm that this is the correct approach, I'll test the patch a >> little bit more and then I'll send a properly signed-off patch; otherwise, if >> someone can suggest the correct approach I'll try it. > > wake_up_new_task() > -> __task_rq_lock() > -> task_woken() > -> push_dl/rt_tasks() > -> push_dl/rt_task() > > I think you also should consider the lockdep pin_lock in this path. Well, I never triggered this warning for the task_woken() path, but now I see it... Thanks!
If someone can confirm that unpinning before calling push/pull and pinning again after the call is the correct thing to do, I'll send a proper patch taking into account all the paths... But for the moment I am still not sure if unpinning the lock here is ok.
Thanks, Luca
| |