Re: lockdep-related warning in kernel/sched/deadline.c::find_lock_later_rq()

[Luca Abeni]

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