[patch] lock validator: rtmutex unlock order annotation

[patch] lock validator: rtmutex unlock order annotation

[Ingo Molnar]

Subject: lock validator: rtmutex unlock order annotation
From: Ingo Molnar <mingo@elte.hu>

rtmutex.c does a tricky piece of 'lock chain' logic spiced with trylock,
which has one particular codepath where we intentionally release the
locks in a different order as acquired. Annotate this for the lock
validator to not complain if CONFIG_DEBUG_NON_NESTED_UNLOCKS=y.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/rtmutex.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux/kernel/rtmutex.c
===================================================================
--- linux.orig/kernel/rtmutex.c
+++ linux/kernel/rtmutex.c
@@ -243,7 +243,8 @@ static int rt_mutex_adjust_prio_chain(ta
 	plist_add(&waiter->list_entry, &lock->wait_list);
 
 	/* Release the task */
-	spin_unlock_irqrestore(&task->pi_lock, flags);
+	spin_unlock_non_nested(&task->pi_lock);
+	local_irq_restore(flags);
 	put_task_struct(task);
 
 	/* Grab the next task */

Re: [patch] lock validator: rtmutex unlock order annotation

[Andrew Morton]

On Thu, 22 Jun 2006 10:57:06 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> rtmutex.c does a tricky piece of 'lock chain' logic spiced with trylock,
> which has one particular codepath where we intentionally release the
> locks in a different order as acquired. Annotate this for the lock
> validator to not complain if CONFIG_DEBUG_NON_NESTED_UNLOCKS=y.
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  kernel/rtmutex.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux/kernel/rtmutex.c
> ===================================================================
> --- linux.orig/kernel/rtmutex.c
> +++ linux/kernel/rtmutex.c
> @@ -243,7 +243,8 @@ static int rt_mutex_adjust_prio_chain(ta
>  	plist_add(&waiter->list_entry, &lock->wait_list);
>  
>  	/* Release the task */
> -	spin_unlock_irqrestore(&task->pi_lock, flags);
> +	spin_unlock_non_nested(&task->pi_lock);
> +	local_irq_restore(flags);

y'know, if an innocent civilian were to stumble across this code he or she
would wonder "wtf is that doing"?

He or she would then go to the definition of spin_unlock_non_nested() and
would find it to be 100% comment-free.

IOW, hasty hackery is flying in all directions and the kernel is getting
crappier as a result.

And that's OK for now, because at some stage I'll be dropping the whole lot
so you can redo the series.  But I just don't know what's in there any more
and we'll need to re-changelog, re-document, re-comment and re-review
everything in the for-real patch series.  (ISTR having some minor comments
and questions against the initial patch series?)

We'll need to review it for comprehensibility (hence maintainability).

We'll need to measure the configged-off code size increase.

I think we have quite a long way to go yet on this stuff.

Re: [patch] lock validator: rtmutex unlock order annotation

[Ingo Molnar]

* Andrew Morton <akpm@osdl.org> wrote:

> > +	spin_unlock_non_nested(&task->pi_lock);
> > +	local_irq_restore(flags);
> 
> y'know, if an innocent civilian were to stumble across this code he or 
> she would wonder "wtf is that doing"?

most definitely! I'll clean that up (and the other items as well).

	Ingo