Skip to content

Commit de92c8c

Browse files
jankaratytso
authored andcommitted
jbd2: speedup jbd2_journal_get_[write|undo]_access()
jbd2_journal_get_write_access() and jbd2_journal_get_create_access() are frequently called for buffers that are already part of the running transaction - most frequently it is the case for bitmaps, inode table blocks, and superblock. Since in such cases we have nothing to do, it is unfortunate we still grab reference to journal head, lock the bh, lock bh_state only to find out there's nothing to do. Improving this is a bit subtle though since until we find out journal head is attached to the running transaction, it can disappear from under us because checkpointing / commit decided it's no longer needed. We deal with this by protecting journal_head slab with RCU. We still have to be careful about journal head being freed & reallocated within slab and about exposing journal head in consistent state (in particular b_modified and b_frozen_data must be in correct state before we allow user to touch the buffer). Signed-off-by: Jan Kara <jack@suse.cz> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
1 parent 8b00f40 commit de92c8c

File tree

2 files changed

+73
-5
lines changed

2 files changed

+73
-5
lines changed

fs/jbd2/journal.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2321,7 +2321,7 @@ static int jbd2_journal_init_journal_head_cache(void)
23212321
jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
23222322
sizeof(struct journal_head),
23232323
0, /* offset */
2324-
SLAB_TEMPORARY, /* flags */
2324+
SLAB_TEMPORARY | SLAB_DESTROY_BY_RCU,
23252325
NULL); /* ctor */
23262326
retval = 0;
23272327
if (!jbd2_journal_head_cache) {

fs/jbd2/transaction.c

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,12 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
892892
JBUFFER_TRACE(jh, "no transaction");
893893
J_ASSERT_JH(jh, !jh->b_next_transaction);
894894
JBUFFER_TRACE(jh, "file as BJ_Reserved");
895+
/*
896+
* Make sure all stores to jh (b_modified, b_frozen_data) are
897+
* visible before attaching it to the running transaction.
898+
* Paired with barrier in jbd2_write_access_granted()
899+
*/
900+
smp_wmb();
895901
spin_lock(&journal->j_list_lock);
896902
__jbd2_journal_file_buffer(jh, transaction, BJ_Reserved);
897903
spin_unlock(&journal->j_list_lock);
@@ -904,8 +910,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
904910
if (jh->b_frozen_data) {
905911
JBUFFER_TRACE(jh, "has frozen data");
906912
J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
907-
jh->b_next_transaction = transaction;
908-
goto done;
913+
goto attach_next;
909914
}
910915

911916
JBUFFER_TRACE(jh, "owned by older transaction");
@@ -959,6 +964,13 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
959964
frozen_buffer = NULL;
960965
jbd2_freeze_jh_data(jh);
961966
}
967+
attach_next:
968+
/*
969+
* Make sure all stores to jh (b_modified, b_frozen_data) are visible
970+
* before attaching it to the running transaction. Paired with barrier
971+
* in jbd2_write_access_granted()
972+
*/
973+
smp_wmb();
962974
jh->b_next_transaction = transaction;
963975

964976
done:
@@ -978,6 +990,55 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
978990
return error;
979991
}
980992

993+
/* Fast check whether buffer is already attached to the required transaction */
994+
static bool jbd2_write_access_granted(handle_t *handle, struct buffer_head *bh)
995+
{
996+
struct journal_head *jh;
997+
bool ret = false;
998+
999+
/* Dirty buffers require special handling... */
1000+
if (buffer_dirty(bh))
1001+
return false;
1002+
1003+
/*
1004+
* RCU protects us from dereferencing freed pages. So the checks we do
1005+
* are guaranteed not to oops. However the jh slab object can get freed
1006+
* & reallocated while we work with it. So we have to be careful. When
1007+
* we see jh attached to the running transaction, we know it must stay
1008+
* so until the transaction is committed. Thus jh won't be freed and
1009+
* will be attached to the same bh while we run. However it can
1010+
* happen jh gets freed, reallocated, and attached to the transaction
1011+
* just after we get pointer to it from bh. So we have to be careful
1012+
* and recheck jh still belongs to our bh before we return success.
1013+
*/
1014+
rcu_read_lock();
1015+
if (!buffer_jbd(bh))
1016+
goto out;
1017+
/* This should be bh2jh() but that doesn't work with inline functions */
1018+
jh = READ_ONCE(bh->b_private);
1019+
if (!jh)
1020+
goto out;
1021+
if (jh->b_transaction != handle->h_transaction &&
1022+
jh->b_next_transaction != handle->h_transaction)
1023+
goto out;
1024+
/*
1025+
* There are two reasons for the barrier here:
1026+
* 1) Make sure to fetch b_bh after we did previous checks so that we
1027+
* detect when jh went through free, realloc, attach to transaction
1028+
* while we were checking. Paired with implicit barrier in that path.
1029+
* 2) So that access to bh done after jbd2_write_access_granted()
1030+
* doesn't get reordered and see inconsistent state of concurrent
1031+
* do_get_write_access().
1032+
*/
1033+
smp_mb();
1034+
if (unlikely(jh->b_bh != bh))
1035+
goto out;
1036+
ret = true;
1037+
out:
1038+
rcu_read_unlock();
1039+
return ret;
1040+
}
1041+
9811042
/**
9821043
* int jbd2_journal_get_write_access() - notify intent to modify a buffer for metadata (not data) update.
9831044
* @handle: transaction to add buffer modifications to
@@ -991,9 +1052,13 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
9911052

9921053
int jbd2_journal_get_write_access(handle_t *handle, struct buffer_head *bh)
9931054
{
994-
struct journal_head *jh = jbd2_journal_add_journal_head(bh);
1055+
struct journal_head *jh;
9951056
int rc;
9961057

1058+
if (jbd2_write_access_granted(handle, bh))
1059+
return 0;
1060+
1061+
jh = jbd2_journal_add_journal_head(bh);
9971062
/* We do not want to get caught playing with fields which the
9981063
* log thread also manipulates. Make sure that the buffer
9991064
* completes any outstanding IO before proceeding. */
@@ -1123,11 +1188,14 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
11231188
int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
11241189
{
11251190
int err;
1126-
struct journal_head *jh = jbd2_journal_add_journal_head(bh);
1191+
struct journal_head *jh;
11271192
char *committed_data = NULL;
11281193

11291194
JBUFFER_TRACE(jh, "entry");
1195+
if (jbd2_write_access_granted(handle, bh))
1196+
return 0;
11301197

1198+
jh = jbd2_journal_add_journal_head(bh);
11311199
/*
11321200
* Do this first --- it can drop the journal lock, so we want to
11331201
* make sure that obtaining the committed_data is done

0 commit comments

Comments
 (0)