All of lore.kernel.org
 help / color / mirror / Atom feed
From: josh@joshtriplett.org
To: Brian Norris <computersforpeace@gmail.com>
Cc: Fabian Frederick <fabf@skynet.be>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-sparse@vger.kernel.org, linux-mtd@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] jffs2: fix sparse warning: unexpected unlock
Date: Fri, 26 Sep 2014 16:17:27 -0700	[thread overview]
Message-ID: <20140926231727.GD20917@cloud> (raw)
In-Reply-To: <20140922181250.GN1193@ld-irv-0074>

On Mon, Sep 22, 2014 at 11:12:50AM -0700, Brian Norris wrote:
> + linux-sparse
> 
> On Thu, Sep 18, 2014 at 08:46:16PM +0200, Fabian Frederick wrote:
> > fs/jffs2/summary.c:846:5: warning: context imbalance in 'jffs2_sum_write_sumnode' - unexpected unlock
> > 
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> >  fs/jffs2/summary.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
> > index c522d09..a0bac7b 100644
> > --- a/fs/jffs2/summary.c
> > +++ b/fs/jffs2/summary.c
> > @@ -844,6 +844,8 @@ static int jffs2_sum_write_data(struct jffs2_sb_info *c, struct jffs2_eraseblock
> >  /* Write out summary information - called from jffs2_do_reserve_space */
> >  
> >  int jffs2_sum_write_sumnode(struct jffs2_sb_info *c)
> > +	__releases(&c->erase_completion_lock)
> > +	__acquires(&c->erase_completion_lock)
> 
> I'm not too familiar with sparse notations, but Documentation/sparse.txt
> suggests the above is wrong, and the following is more accurate:
> 
> 	__must_hold(&c->erase_completion_lock)
> 
> But it looks like there are several other examples which do this.
> Anyway, here's the relevant doc text, in case someone wants to clarify
> it for me, or else tell me the documentation is wrong:
> 
>     __must_hold - The specified lock is held on function entry and exit.
> 
>     __acquires - The specified lock is held on function exit, but not entry.
> 
>     __releases - The specified lock is held on function entry, but not exit.
> 
> So __acquires and __releases look mutually exclusive, but it's not clear
> if __must_hold will actually cover what we want. (I haven't tested it.)

__must_hold is indeed the correct annotation.  (There isn't currently
anything enforcing that, though.)

- Josh Triplett

WARNING: multiple messages have this Message-ID (diff)
From: josh@joshtriplett.org
To: Brian Norris <computersforpeace@gmail.com>
Cc: Fabian Frederick <fabf@skynet.be>,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	David Woodhouse <dwmw2@infradead.org>,
	linux-sparse@vger.kernel.org
Subject: Re: [PATCH 1/1] jffs2: fix sparse warning: unexpected unlock
Date: Fri, 26 Sep 2014 16:17:27 -0700	[thread overview]
Message-ID: <20140926231727.GD20917@cloud> (raw)
In-Reply-To: <20140922181250.GN1193@ld-irv-0074>

On Mon, Sep 22, 2014 at 11:12:50AM -0700, Brian Norris wrote:
> + linux-sparse
> 
> On Thu, Sep 18, 2014 at 08:46:16PM +0200, Fabian Frederick wrote:
> > fs/jffs2/summary.c:846:5: warning: context imbalance in 'jffs2_sum_write_sumnode' - unexpected unlock
> > 
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> >  fs/jffs2/summary.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
> > index c522d09..a0bac7b 100644
> > --- a/fs/jffs2/summary.c
> > +++ b/fs/jffs2/summary.c
> > @@ -844,6 +844,8 @@ static int jffs2_sum_write_data(struct jffs2_sb_info *c, struct jffs2_eraseblock
> >  /* Write out summary information - called from jffs2_do_reserve_space */
> >  
> >  int jffs2_sum_write_sumnode(struct jffs2_sb_info *c)
> > +	__releases(&c->erase_completion_lock)
> > +	__acquires(&c->erase_completion_lock)
> 
> I'm not too familiar with sparse notations, but Documentation/sparse.txt
> suggests the above is wrong, and the following is more accurate:
> 
> 	__must_hold(&c->erase_completion_lock)
> 
> But it looks like there are several other examples which do this.
> Anyway, here's the relevant doc text, in case someone wants to clarify
> it for me, or else tell me the documentation is wrong:
> 
>     __must_hold - The specified lock is held on function entry and exit.
> 
>     __acquires - The specified lock is held on function exit, but not entry.
> 
>     __releases - The specified lock is held on function entry, but not exit.
> 
> So __acquires and __releases look mutually exclusive, but it's not clear
> if __must_hold will actually cover what we want. (I haven't tested it.)

__must_hold is indeed the correct annotation.  (There isn't currently
anything enforcing that, though.)

- Josh Triplett

  reply	other threads:[~2014-09-26 23:17 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 18:46 [PATCH 1/1] jffs2: fix sparse warning: unexpected unlock Fabian Frederick
2014-09-18 18:46 ` Fabian Frederick
2014-09-22 18:12 ` Brian Norris
2014-09-22 18:12   ` Brian Norris
2014-09-26 23:17   ` josh [this message]
2014-09-26 23:17     ` josh
2014-09-27  8:41     ` Fabian Frederick
2014-09-27  8:41       ` Fabian Frederick
2014-09-28  9:56       ` Josh Triplett
2014-09-28  9:56         ` Josh Triplett
2014-09-28  9:56         ` Josh Triplett

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=20140926231727.GD20917@cloud \
    --to=josh@joshtriplett.org \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=fabf@skynet.be \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-sparse@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.