Re: [PATCH] wireless: wl12xx, fix lock imbalance

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

On Sat, 2009-07-18 at 13:19 +0200, Ingo Molnar wrote:

> Yes. And in fact 'nice' code wants to be either annotated explicitly 
> as 'I am taking locks', or should be balanced.

I agree.

> I was thinking about also using lockdep plus the function-graph 
> tracer for that (in the dynamic lock debugging department).

Yeah, but that's dynamic again -- all the error paths are never caught.

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

I don't see a need to do it dynamically since sparse warns about things
like this. It's quirky in some ways and I've tried to fix it up before
(and failed) but it's not something that can't be fixed, it just needs
more than a night of hacking.

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

What's "this" in this context?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]