linux-kernel - Re: [patch V3 6/7] rtmutex: Cleanup deadlock detector debug logic

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]

Message-ID: <20140609210458.10c4ac8d@gandalf.local.home>
Date:	Mon, 9 Jun 2014 21:04:58 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Jason Low <jason.low2@...com>, Brad Mouring <bmouring@...com>
Subject: Re: [patch V3 6/7] rtmutex: Cleanup deadlock detector debug logic

On Mon, 09 Jun 2014 20:28:10 -0000
Thomas Gleixner <tglx@...utronix.de> wrote:

> The conditions under which deadlock detection is conducted are unclear
> and undocumented.
> 
> Add constants instead of using 0/1 and provide a selection function
> which hides the additional debug dependency from the calling code.
> 
> Add comments where needed.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  kernel/locking/rtmutex-debug.c  |    5 +-
>  kernel/locking/rtmutex-debug.h  |    7 ++--
>  kernel/locking/rtmutex.c        |   69 +++++++++++++++++++++++++++-------------
>  kernel/locking/rtmutex.h        |    7 +++-
>  kernel/locking/rtmutex_common.h |   15 ++++++++
>  5 files changed, 75 insertions(+), 28 deletions(-)
> 
> Index: tip/kernel/locking/rtmutex-debug.c
> ===================================================================
> --- tip.orig/kernel/locking/rtmutex-debug.c
> +++ tip/kernel/locking/rtmutex-debug.c
> @@ -66,12 +66,13 @@ void rt_mutex_debug_task_free(struct tas
>   * the deadlock. We print when we return. act_waiter can be NULL in
>   * case of a remove waiter operation.
>   */
> -void debug_rt_mutex_deadlock(int detect, struct rt_mutex_waiter *act_waiter,
> +void debug_rt_mutex_deadlock(enum rtmutex_chainwalk chwalk,
> +			     struct rt_mutex_waiter *act_waiter,
>  			     struct rt_mutex *lock)
>  {
>  	struct task_struct *task;
>  
> -	if (!debug_locks || detect || !act_waiter)
> +	if (!debug_locks || chwalk || !act_waiter)

I know this will probably get a little verbose, but chwalk isn't very
descriptive. Perhaps change this to:

	if (!debug_locks || chwalk == RT_MUTEX_FULL_CHAINWALK ||
	    !act_waiter)

To cut down on the verbosity, we could add helper macros:

#define chwalk_is_full(chwalk)	((chwalk) == RT_MUTEX_FULL_CHAINWALK)
#define chwalk_is_min(chwalk)   ((chwalk) == RT_MUTEX_MIN_CHAINWALK)

And then the above would simply be:

	if (!debug_locks || chwalk_is_full(chwalk) || !act_waiter)

And put this throughout.

-- Steve

--
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/