Skip to content

Commit

Permalink
jbd2: Make state lock a spinlock
Browse files Browse the repository at this point in the history
Bit-spinlocks are problematic on PREEMPT_RT if functions which might sleep
on RT, e.g. spin_lock(), alloc/free(), are invoked inside the lock held
region because bit spinlocks disable preemption even on RT.

A first attempt was to replace state lock with a spinlock placed in struct
buffer_head and make the locking conditional on PREEMPT_RT and
DEBUG_BIT_SPINLOCKS.

Jan pointed out that there is a 4 byte hole in struct journal_head where a
regular spinlock fits in and he would not object to convert the state lock
to a spinlock unconditionally.

Aside of solving the RT problem, this also gains lockdep coverage for the
journal head state lock (bit-spinlocks are not covered by lockdep as it's
hard to fit a lockdep map into a single bit).

The trivial change would have been to convert the jbd_*lock_bh_state()
inlines, but that comes with the downside that these functions take a
buffer head pointer which needs to be converted to a journal head pointer
which adds another level of indirection.

As almost all functions which use this lock have a journal head pointer
readily available, it makes more sense to remove the lock helper inlines
and write out spin_*lock() at all call sites.

Fixup all locking comments as well.

Suggested-by: Jan Kara <jack@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jan Kara <jack@suse.cz>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Jan Kara <jack@suse.com>
Cc: linux-ext4@vger.kernel.org
Link: https://lore.kernel.org/r/20190809124233.13277-7-jack@suse.cz
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  • Loading branch information
Thomas Gleixner authored and Theodore Ts'o committed Oct 21, 2019
1 parent 2e710ff commit 4641706
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 94 deletions.
8 changes: 4 additions & 4 deletions fs/jbd2/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,10 @@ void jbd2_journal_commit_transaction(journal_t *journal)
if (jh->b_committed_data) {
struct buffer_head *bh = jh2bh(jh);

jbd_lock_bh_state(bh);
spin_lock(&jh->b_state_lock);
jbd2_free(jh->b_committed_data, bh->b_size);
jh->b_committed_data = NULL;
jbd_unlock_bh_state(bh);
spin_unlock(&jh->b_state_lock);
}
jbd2_journal_refile_buffer(journal, jh);
}
Expand Down Expand Up @@ -928,7 +928,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
* done with it.
*/
get_bh(bh);
jbd_lock_bh_state(bh);
spin_lock(&jh->b_state_lock);
J_ASSERT_JH(jh, jh->b_transaction == commit_transaction);

/*
Expand Down Expand Up @@ -1024,7 +1024,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
}
JBUFFER_TRACE(jh, "refile or unfile buffer");
drop_ref = __jbd2_journal_refile_buffer(jh);
jbd_unlock_bh_state(bh);
spin_unlock(&jh->b_state_lock);
if (drop_ref)
jbd2_journal_put_journal_head(jh);
if (try_to_free)
Expand Down
10 changes: 6 additions & 4 deletions fs/jbd2/journal.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
/* keep subsequent assertions sane */
atomic_set(&new_bh->b_count, 1);

jbd_lock_bh_state(bh_in);
spin_lock(&jh_in->b_state_lock);
repeat:
/*
* If a new transaction has already done a buffer copy-out, then
Expand Down Expand Up @@ -405,13 +405,13 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
if (need_copy_out && !done_copy_out) {
char *tmp;

jbd_unlock_bh_state(bh_in);
spin_unlock(&jh_in->b_state_lock);
tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS);
if (!tmp) {
brelse(new_bh);
return -ENOMEM;
}
jbd_lock_bh_state(bh_in);
spin_lock(&jh_in->b_state_lock);
if (jh_in->b_frozen_data) {
jbd2_free(tmp, bh_in->b_size);
goto repeat;
Expand Down Expand Up @@ -464,7 +464,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
__jbd2_journal_file_buffer(jh_in, transaction, BJ_Shadow);
spin_unlock(&journal->j_list_lock);
set_buffer_shadow(bh_in);
jbd_unlock_bh_state(bh_in);
spin_unlock(&jh_in->b_state_lock);

return do_escape | (done_copy_out << 1);
}
Expand Down Expand Up @@ -2410,6 +2410,8 @@ static struct journal_head *journal_alloc_journal_head(void)
ret = kmem_cache_zalloc(jbd2_journal_head_cache,
GFP_NOFS | __GFP_NOFAIL);
}
if (ret)
spin_lock_init(&ret->b_state_lock);
return ret;
}

Expand Down
Loading

0 comments on commit 4641706

Please sign in to comment.