* [patch 3/9] input: ads7846.c sparse lock annotation
@ 2009-05-12 20:43 akpm
2009-05-14 3:24 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: akpm @ 2009-05-12 20:43 UTC (permalink / raw)
To: dtor; +Cc: linux-input, akpm, harvey.harrison
From: Harvey Harrison <harvey.harrison@gmail.com>
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
Cc: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/input/touchscreen/ads7846.c | 2 ++
1 file changed, 2 insertions(+)
diff -puN drivers/input/touchscreen/ads7846.c~input-ads7846c-sparse-lock-annotation drivers/input/touchscreen/ads7846.c
--- a/drivers/input/touchscreen/ads7846.c~input-ads7846c-sparse-lock-annotation
+++ a/drivers/input/touchscreen/ads7846.c
@@ -769,6 +769,8 @@ static irqreturn_t ads7846_irq(int irq,
/* Must be called with ts->lock held */
static void ads7846_disable(struct ads7846 *ts)
+__releases(&ts->lock)
+__acquires(&ts->lock)
{
if (ts->disabled)
return;
_
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 3/9] input: ads7846.c sparse lock annotation
2009-05-12 20:43 [patch 3/9] input: ads7846.c sparse lock annotation akpm
@ 2009-05-14 3:24 ` Dmitry Torokhov
2009-05-14 3:34 ` Harvey Harrison
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2009-05-14 3:24 UTC (permalink / raw)
To: akpm; +Cc: linux-input, harvey.harrison
On Tue, May 12, 2009 at 01:43:06PM -0700, akpm@linux-foundation.org wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
>
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> drivers/input/touchscreen/ads7846.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff -puN drivers/input/touchscreen/ads7846.c~input-ads7846c-sparse-lock-annotation drivers/input/touchscreen/ads7846.c
> --- a/drivers/input/touchscreen/ads7846.c~input-ads7846c-sparse-lock-annotation
> +++ a/drivers/input/touchscreen/ads7846.c
> @@ -769,6 +769,8 @@ static irqreturn_t ads7846_irq(int irq,
>
> /* Must be called with ts->lock held */
> static void ads7846_disable(struct ads7846 *ts)
> +__releases(&ts->lock)
> +__acquires(&ts->lock)
> {
I still haven't gotten any explanation why this is needed and also I am
still getting sparce warnings with this patch applied. Please drop.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 3/9] input: ads7846.c sparse lock annotation
2009-05-14 3:24 ` Dmitry Torokhov
@ 2009-05-14 3:34 ` Harvey Harrison
2009-05-14 3:51 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Harvey Harrison @ 2009-05-14 3:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: akpm, linux-input
On Wed, 2009-05-13 at 20:24 -0700, Dmitry Torokhov wrote:
> On Tue, May 12, 2009 at 01:43:06PM -0700, akpm@linux-foundation.org wrote:
> > From: Harvey Harrison <harvey.harrison@gmail.com>
> >
> > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > Cc: Dmitry Torokhov <dtor@mail.ru>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> > /* Must be called with ts->lock held */
> > static void ads7846_disable(struct ads7846 *ts)
> > +__releases(&ts->lock)
> > +__acquires(&ts->lock)
> > {
>
> I still haven't gotten any explanation why this is needed and also I am
> still getting sparce warnings with this patch applied. Please drop.
>
Sorry, I didn't realize I had a local patchset doing extra context checking
of what lock is passed in to spin_lock/unlock...it helps to document when a
function requires a lock held (see the comment)
The culprit in this function:
while (ts->pending) {
spin_unlock_irq(&ts->lock);
msleep(1);
spin_lock_irq(&ts->lock);
}
Anyways, as current sparse will still warn please drop.
Harvey
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 3/9] input: ads7846.c sparse lock annotation
2009-05-14 3:34 ` Harvey Harrison
@ 2009-05-14 3:51 ` Dmitry Torokhov
2009-05-14 14:45 ` Jiri Kosina
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2009-05-14 3:51 UTC (permalink / raw)
To: Harvey Harrison; +Cc: akpm, linux-input
On Wed, May 13, 2009 at 08:34:27PM -0700, Harvey Harrison wrote:
> On Wed, 2009-05-13 at 20:24 -0700, Dmitry Torokhov wrote:
> > On Tue, May 12, 2009 at 01:43:06PM -0700, akpm@linux-foundation.org wrote:
> > > From: Harvey Harrison <harvey.harrison@gmail.com>
> > >
> > > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > > Cc: Dmitry Torokhov <dtor@mail.ru>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > > ---
> > >
> > > /* Must be called with ts->lock held */
> > > static void ads7846_disable(struct ads7846 *ts)
> > > +__releases(&ts->lock)
> > > +__acquires(&ts->lock)
> > > {
> >
> > I still haven't gotten any explanation why this is needed and also I am
> > still getting sparce warnings with this patch applied. Please drop.
> >
>
> Sorry, I didn't realize I had a local patchset doing extra context checking
> of what lock is passed in to spin_lock/unlock...it helps to document when a
> function requires a lock held (see the comment)
>
I understand the comment ;) The annotation syntax is confusing though...
Maybe if instead of __releases and __acquires it said __needs_lock() and
__leaves_locked() that would make more sense...
> The culprit in this function:
> while (ts->pending) {
> spin_unlock_irq(&ts->lock);
> msleep(1);
> spin_lock_irq(&ts->lock);
> }
>
> Anyways, as current sparse will still warn please drop.
>
> Harvey
>
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 3/9] input: ads7846.c sparse lock annotation
2009-05-14 3:51 ` Dmitry Torokhov
@ 2009-05-14 14:45 ` Jiri Kosina
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2009-05-14 14:45 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Harvey Harrison, akpm, linux-input
On Wed, 13 May 2009, Dmitry Torokhov wrote:
> > > > /* Must be called with ts->lock held */
> > > > static void ads7846_disable(struct ads7846 *ts)
> > > > +__releases(&ts->lock)
> > > > +__acquires(&ts->lock)
> > > > {
> > > I still haven't gotten any explanation why this is needed and also I am
> > > still getting sparce warnings with this patch applied. Please drop.
> > Sorry, I didn't realize I had a local patchset doing extra context
> > checking of what lock is passed in to spin_lock/unlock...it helps to
> > document when a function requires a lock held (see the comment)
> I understand the comment ;) The annotation syntax is confusing though...
> Maybe if instead of __releases and __acquires it said __needs_lock() and
> __leaves_locked() that would make more sense...
I fully agree. __acquires() and __releases() is used to annotate a
completely different thing -- it should be used solely to avoid false
positives about locking inbalances across functions (i.e. one function
deliberately not unlocking lock that it has locked), shoudldn't it?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-05-14 14:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-12 20:43 [patch 3/9] input: ads7846.c sparse lock annotation akpm
2009-05-14 3:24 ` Dmitry Torokhov
2009-05-14 3:34 ` Harvey Harrison
2009-05-14 3:51 ` Dmitry Torokhov
2009-05-14 14:45 ` Jiri Kosina
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.