From: Ingo Molnar <mingo@elte.hu>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
Jiri Slaby <jirislaby@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] wireless: wl12xx, fix lock imbalance
Date: Sat, 18 Jul 2009 18:10:29 +0200 [thread overview]
Message-ID: <20090718161029.GA16343@elte.hu> (raw)
In-Reply-To: <1247916829.1055.24.camel@johannes.local>
* Johannes Berg <johannes@sipsolutions.net> wrote:
> > 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.
Yeah - but Sparse warns about this if it can analyze the code path.
If it cannot see through it then it cannot warn. Static analysis
will go only that far - dynamic analysis will catch the cases that
do happen.
So it's best to have both: static analysis is good at finding
imbalances even if they have a very low likelyhood of occuring in
practice, while dynamic analysis will catch everything that does
trigger in practice, regardless of code flow complexity.
( The only white area on the map is rarely executed code that has a
complex code flow. Such code is being frowned upon in general at
the review stage. )
> > 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?
this == stanse, the static code analyzing thing Jiri mentioned he is
working on. The webpage says it will be under the GPL - that's good.
Jiri, any release date for the source code?
Ingo
next prev parent reply other threads:[~2009-07-18 16:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-13 21:24 [PATCH] wireless: wl12xx, fix lock imbalance Jiri Slaby
2009-07-13 21:40 ` Johannes Berg
2009-07-13 21:44 ` Jiri Slaby
2009-07-13 21:49 ` Jiri Slaby
2009-07-13 21:49 ` Johannes Berg
2009-07-13 21:51 ` Jiri Slaby
2009-07-13 21:54 ` Johannes Berg
2009-07-18 11:19 ` Ingo Molnar
2009-07-18 11:33 ` Johannes Berg
2009-07-18 16:10 ` Ingo Molnar [this message]
2009-07-26 8:00 ` stanse [was: wireless: wl12xx, fix lock imbalance] Jiri Slaby
2009-10-12 10:11 ` Stanse 1.0.0 released " Jiri Slaby
2009-10-12 10:11 ` Jiri Slaby
2009-10-12 10:47 ` Stanse 1.0.0 released [was: wireless: wl12xx, fix lock Ingo Molnar
2009-10-12 10:47 ` Stanse 1.0.0 released [was: wireless: wl12xx, fix lock imbalance] Ingo Molnar
2009-07-14 5:44 ` [PATCH] wireless: wl12xx, fix lock imbalance Luciano Coelho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090718161029.GA16343@elte.hu \
--to=mingo@elte.hu \
--cc=a.p.zijlstra@chello.nl \
--cc=jirislaby@gmail.com \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.