From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756775AbbJVHGk (ORCPT ); Thu, 22 Oct 2015 03:06:40 -0400 Received: from mail-wi0-f171.google.com ([209.85.212.171]:37450 "EHLO mail-wi0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751544AbbJVHGi (ORCPT ); Thu, 22 Oct 2015 03:06:38 -0400 Subject: Re: lockdep-related warning in kernel/sched/deadline.c::find_lock_later_rq() To: Wanpeng Li , linux-kernel@vger.kernel.org References: <56279107.3010602@unitn.it> Cc: Peter Zijlstra , Ingo Molnar , Juri Lelli From: Luca Abeni Message-ID: <56288AF7.20602@unitn.it> Date: Thu, 22 Oct 2015 09:06:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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