2014-09-18 18:29:34

by Fabian Frédérick

Subject: [PATCH 1/1 linux-next] jbd2: Fix sparse context imbalance warning

spin_unlock after spin_lock only.

This fixes the following sparse warning:
fs/jbd2/transaction.c:1102:20: warning: context imbalance
in 'jbd2_journal_get_create_access' - different lock contexts for basic block

Signed-off-by: Fabian Frederick <[email protected]>
---
fs/jbd2/transaction.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 5f09370..edb7f59 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1091,6 +1091,7 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
JBUFFER_TRACE(jh, "file as BJ_Reserved");
spin_lock(&journal->j_list_lock);
__jbd2_journal_file_buffer(jh, transaction, BJ_Reserved);
+ spin_unlock(&journal->j_list_lock);
} else if (jh->b_transaction == journal->j_committing_transaction) {
/* first access by this transaction */
jh->b_modified = 0;
@@ -1098,8 +1099,8 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
JBUFFER_TRACE(jh, "set next transaction");
spin_lock(&journal->j_list_lock);
jh->b_next_transaction = transaction;
+ spin_unlock(&journal->j_list_lock);
}
- spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);

/*
--
1.9.1

2014-09-18 20:44:55

by Theodore Ts'o

Subject: Re: [PATCH 1/1 linux-next] jbd2: Fix sparse context imbalance warning

On Thu, Sep 18, 2014 at 08:29:34PM +0200, Fabian Frederick wrote:
> spin_unlock after spin_lock only.
>
> This fixes the following sparse warning:
> fs/jbd2/transaction.c:1102:20: warning: context imbalance
> in 'jbd2_journal_get_create_access' - different lock contexts for basic block
>
> Signed-off-by: Fabian Frederick <[email protected]>

NAK, this is a case where I think it's better to make the static
analyzers better. In any case sparse gives a lot of false positives
for this particular warning, so adding lines of code to try to shut up
sparse for every single false positive seems to be a losing
proposition.

I'll note that these sorts of bugs are much more easily picked up
using lockdep in any case, so we're probably better of just ignoring
these sorts of warnings. Better to let the 0day kernel tester warn
for new sparse warnings, instead of trying to drive sparse warnings
down to zero.

Cheers,

- Ted

2014-09-20 07:14:11

by Fabian Frédérick

Subject: Re: [PATCH 1/1 linux-next] jbd2: Fix sparse context imbalance warning

> On 18 September 2014 at 22:44 Theodore Ts'o <[email protected]> wrote:
>
>
> On Thu, Sep 18, 2014 at 08:29:34PM +0200, Fabian Frederick wrote:
> > spin_unlock after spin_lock only.
> >
> > This fixes the following sparse warning:
> > fs/jbd2/transaction.c:1102:20: warning: context imbalance
> > in 'jbd2_journal_get_create_access' - different lock contexts for basic
> > block
> >
> > Signed-off-by: Fabian Frederick <[email protected]>
>
> NAK, this is a case where I think it's better to make the static
> analyzers better.  In any case sparse gives a lot of false positives
> for this particular warning, so adding lines of code to try to shut up
> sparse for every single false positive seems to be a losing
> proposition.
>
> I'll note that these sorts of bugs are much more easily picked up
> using lockdep in any case, so we're probably better of just ignoring
> these sorts of warnings.  Better to let the 0day kernel tester warn
> for new sparse warnings, instead of trying to drive sparse warnings
> down to zero.
>
> Cheers,
>
>                                       - Ted

Thanks a lot for explanation Ted. I didn't see J_ASSERT_JH above which BUG_ON
if b_transaction is not ok.


Regards,
Fabian