Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754630AbaFJDsv (ORCPT ); Mon, 9 Jun 2014 23:48:51 -0400 Received: from mail-ie0-f181.google.com ([209.85.223.181]:59124 "EHLO mail-ie0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754430AbaFJDsu (ORCPT ); Mon, 9 Jun 2014 23:48:50 -0400 MIME-Version: 1.0 In-Reply-To: <20140609212033.1cff0999@gandalf.local.home> References: <20140609201118.387571774@linutronix.de> <20140609202336.506044876@linutronix.de> <20140609212033.1cff0999@gandalf.local.home> Date: Mon, 9 Jun 2014 20:48:49 -0700 X-Google-Sender-Auth: LRGiHhgLo35BVs782OcF8AvE0CI Message-ID: Subject: Re: [patch V3 7/7] rtmutex: Avoid pointless requeueing in the deadlock detection chain walk From: Jason Low To: Steven Rostedt Cc: Thomas Gleixner , LKML , Peter Zijlstra , Ingo Molnar , Lai Jiangshan , Jason Low , Brad Mouring Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 9, 2014 at 6:20 PM, Steven Rostedt wrote: > On Mon, 09 Jun 2014 20:28:10 -0000 > Thomas Gleixner wrote: > >> In case the dead lock detector is enabled we follow the lock chain to >> the end in rt_mutex_adjust_prio_chain, even if we could stop earlier >> due to the priority/waiter constellation. >> >> But once we are not longer the top priority waiter in a certain step >> or the task holding the lock has already the same priority then there >> is no point in dequeing and enqueing along the lock chain as there is >> no change at all. >> >> So stop the requeueing at this point. >> >> Signed-off-by: Thomas Gleixner >> --- >> kernel/locking/rtmutex.c | 61 +++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 54 insertions(+), 7 deletions(-) >> >> Index: tip/kernel/locking/rtmutex.c >> =================================================================== >> --- tip.orig/kernel/locking/rtmutex.c >> +++ tip/kernel/locking/rtmutex.c >> @@ -359,6 +359,7 @@ static int rt_mutex_adjust_prio_chain(st >> struct rt_mutex *lock; >> bool detect_deadlock; >> unsigned long flags; >> + bool requeue = true; >> >> detect_deadlock = rt_mutex_cond_detect_deadlock(orig_waiter, chwalk); >> >> @@ -436,18 +437,31 @@ static int rt_mutex_adjust_prio_chain(st >> goto out_unlock_pi; >> /* >> * If deadlock detection is off, we stop here if we >> - * are not the top pi waiter of the task. >> + * are not the top pi waiter of the task. If deadlock >> + * detection is enabled we continue, but stop the >> + * requeueing in the chain walk. >> */ >> - if (!detect_deadlock && top_waiter != task_top_pi_waiter(task)) >> - goto out_unlock_pi; >> + if (top_waiter != task_top_pi_waiter(task)) { >> + if (!detect_deadlock) >> + goto out_unlock_pi; >> + else >> + requeue = false; >> + } >> } >> >> /* >> - * When deadlock detection is off then we check, if further >> - * priority adjustment is necessary. >> + * If the waiter priority is the same as the task priority >> + * then there is no further priority adjustment necessary. If >> + * deadlock detection is off, we stop the chain walk. If its >> + * enabled we continue, but stop the requeueing in the chain >> + * walk. >> */ >> - if (!detect_deadlock && waiter->prio == task->prio) >> - goto out_unlock_pi; >> + if (waiter->prio == task->prio) { >> + if (!detect_deadlock) >> + goto out_unlock_pi; >> + else >> + requeue = false; >> + } >> >> /* >> * We need to trylock here as we are holding task->pi_lock, >> @@ -475,6 +489,39 @@ static int rt_mutex_adjust_prio_chain(st >> } >> >> /* >> + * If we just follow the lock chain for deadlock detection, no >> + * need to do all the requeue operations. We avoid a truckload > > s/We/To/ > > >> + * of conditinals around the various places below and just do > > s/ and/, / And s/conditinals/conditionals/ >> + * the minimum chain walk checks here. >> + */ >> + if (!requeue) { >> + /* Release the task */ >> + raw_spin_unlock_irqrestore(&task->pi_lock, flags); >> + put_task_struct(task); >> + >> + /* If there is no owner of the lock, end of chain. */ >> + if (!rt_mutex_owner(lock)) { >> + raw_spin_unlock(&lock->wait_lock); >> + return 0; >> + } >> + >> + /* Grab the next task, i.e. owner of @lock */ >> + task = rt_mutex_owner(lock); >> + get_task_struct(task); >> + raw_spin_lock_irqsave(&task->pi_lock, flags); >> + >> + /* Store whether owner is blocked itself and drop locks */ >> + next_lock = task_blocked_on(task); >> + raw_spin_unlock_irqrestore(&task->pi_lock, flags); >> + raw_spin_unlock(&lock->wait_lock); >> + >> + /* If owner is not blocked, end of chain. */ >> + if (!next_lock) >> + goto out_put_task; > > On the loop back around, have something like: > > if (top_waiter) { > if (!task_has_pi_waiters(task)) > goto out_unlock_pi; > > if (!requeue && > top_waiter != task_top_pi_waiter(task)) { > if (!detect_deadlock) > goto out_unlock_pi; > else > requeue = false; > } > } > > ?? > > >> + goto again; >> + } >> + >> + /* >> * Store the current top waiter before doing the requeue >> * operation on @lock. We need it for the boost/deboost >> * decision below. >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/