linux-kernel - Re: lockdep-related warning in kernel/sched/deadline.c::find_lock_later

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 22 Oct 2015 09:06:31 +0200
From:	Luca Abeni <luca.abeni@...tn.it>
To:	Wanpeng Li <wanpeng.li@...mail.com>, linux-kernel@...r.kernel.org
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@....com>
Subject: Re: lockdep-related warning in
 kernel/sched/deadline.c::find_lock_later_rq()

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/