[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090718111948.GD27287@elte.hu>
Date: Sat, 18 Jul 2009 13:19:49 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Johannes Berg <johannes@...solutions.net>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Jiri Slaby <jirislaby@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] wireless: wl12xx, fix lock imbalance
* Johannes Berg <johannes@...solutions.net> wrote:
> On Mon, 2009-07-13 at 23:51 +0200, Jiri Slaby wrote:
> > On 07/13/2009 11:49 PM, Johannes Berg wrote:
> > > On Mon, 2009-07-13 at 23:44 +0200, Jiri Slaby wrote:
> > >
> > >>> I've had local hacks
> > >>> many times to make sparse aware of mutexes, is there a reason they are
> > >>> not annotated with __acquire(s)/__release(s) like spinlocks etc.?
> > >>
> > >> Mutexes are often locked/unlocked interprocedural which I think sparse
> > >> can't do much about.
> > >
> > > Well, you annotate those functions too, of course.
> >
> > Sorry, I don't understand. What functions I annotate?
>
> Well those that take the mutex, e.g.
>
> void acquire_foo(struct foo *f)
> {
> mutex_lock(&f->mtx);
> }
>
>
> turns to
>
> void acquire_foo(struct foo *f)
> __acquires(f->mtx)
> {
> mutex_lock(&f->mtx);
> }
>
> johannes
Yes. And in fact 'nice' code wants to be either annotated explicitly
as 'I am taking locks', or should be balanced.
I was thinking about also using lockdep plus the function-graph
tracer for that (in the dynamic lock debugging department).
It would work like this: __acquires()/__releases() would also emit
section markers like __lockfunc, and lockdep would warn about
functions that return with unbalanced locks, irqs or preempt counts
and do not declare themselves as locking related functions.
This would help catch imbalances at their source.
Plus static tools like Jiri is working on are very useful as well. I
think Coverty does that too and it's a pity we dont have free tools
for that. In fact Covery will sweep clean the kernel of such bugs,
giving OSS tools like 'stanse' the false impression that there are
no such bugs. There are such bugs - there's a constant influx of
them. So please work on this, it looks very useful.
Ingo
--
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/
Powered by blists - more mailing lists