Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

jbd2: remove bh_state lock from checkpointing code

All accesses to checkpointing entries in journal_head are protected
by j_list_lock. Thus __jbd2_journal_remove_checkpoint() doesn't really
need bh_state lock.

Also the only part of journal head that the rest of checkpointing code
needs to check is jh->b_transaction which is safe to read under
j_list_lock.

So we can safely remove bh_state lock from all of checkpointing code which
makes it considerably prettier.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

authored by

Jan Kara and committed by
Theodore Ts'o
932bb305 c254c9ec

+9 -52
+7 -52
fs/jbd2/checkpoint.c
··· 88 88 * whole transaction. 89 89 * 90 90 * Requires j_list_lock 91 - * Called under jbd_lock_bh_state(jh2bh(jh)), and drops it 92 91 */ 93 92 static int __try_to_free_cp_buf(struct journal_head *jh) 94 93 { 95 94 int ret = 0; 96 95 struct buffer_head *bh = jh2bh(jh); 97 96 98 - if (jh->b_jlist == BJ_None && !buffer_locked(bh) && 97 + if (jh->b_transaction == NULL && !buffer_locked(bh) && 99 98 !buffer_dirty(bh) && !buffer_write_io_error(bh)) { 100 99 /* 101 100 * Get our reference so that bh cannot be freed before ··· 103 104 get_bh(bh); 104 105 JBUFFER_TRACE(jh, "remove from checkpoint list"); 105 106 ret = __jbd2_journal_remove_checkpoint(jh) + 1; 106 - jbd_unlock_bh_state(bh); 107 107 BUFFER_TRACE(bh, "release"); 108 108 __brelse(bh); 109 - } else { 110 - jbd_unlock_bh_state(bh); 111 109 } 112 110 return ret; 113 111 } ··· 176 180 } 177 181 178 182 /* 179 - * We were unable to perform jbd_trylock_bh_state() inside j_list_lock. 180 - * The caller must restart a list walk. Wait for someone else to run 181 - * jbd_unlock_bh_state(). 182 - */ 183 - static void jbd_sync_bh(journal_t *journal, struct buffer_head *bh) 184 - __releases(journal->j_list_lock) 185 - { 186 - get_bh(bh); 187 - spin_unlock(&journal->j_list_lock); 188 - jbd_lock_bh_state(bh); 189 - jbd_unlock_bh_state(bh); 190 - put_bh(bh); 191 - } 192 - 193 - /* 194 183 * Clean up transaction's list of buffers submitted for io. 195 184 * We wait for any pending IO to complete and remove any clean 196 185 * buffers. Note that we take the buffers in the opposite ordering ··· 203 222 while (!released && transaction->t_checkpoint_io_list) { 204 223 jh = transaction->t_checkpoint_io_list; 205 224 bh = jh2bh(jh); 206 - if (!jbd_trylock_bh_state(bh)) { 207 - jbd_sync_bh(journal, bh); 208 - spin_lock(&journal->j_list_lock); 209 - goto restart; 210 - } 211 225 get_bh(bh); 212 226 if (buffer_locked(bh)) { 213 227 spin_unlock(&journal->j_list_lock); 214 - jbd_unlock_bh_state(bh); 215 228 wait_on_buffer(bh); 216 229 /* the journal_head may have gone by now */ 217 230 BUFFER_TRACE(bh, "brelse"); ··· 221 246 * it has been written out and so we can drop it from the list 222 247 */ 223 248 released = __jbd2_journal_remove_checkpoint(jh); 224 - jbd_unlock_bh_state(bh); 225 249 __brelse(bh); 226 250 } 227 251 ··· 254 280 * be written out. 255 281 * 256 282 * Called with j_list_lock held and drops it if 1 is returned 257 - * Called under jbd_lock_bh_state(jh2bh(jh)), and drops it 258 283 */ 259 284 static int __process_buffer(journal_t *journal, struct journal_head *jh, 260 285 int *batch_count, transaction_t *transaction) ··· 264 291 if (buffer_locked(bh)) { 265 292 get_bh(bh); 266 293 spin_unlock(&journal->j_list_lock); 267 - jbd_unlock_bh_state(bh); 268 294 wait_on_buffer(bh); 269 295 /* the journal_head may have gone by now */ 270 296 BUFFER_TRACE(bh, "brelse"); ··· 275 303 276 304 transaction->t_chp_stats.cs_forced_to_close++; 277 305 spin_unlock(&journal->j_list_lock); 278 - jbd_unlock_bh_state(bh); 279 306 if (unlikely(journal->j_flags & JBD2_UNMOUNT)) 280 307 /* 281 308 * The journal thread is dead; so starting and ··· 293 322 if (unlikely(buffer_write_io_error(bh))) 294 323 ret = -EIO; 295 324 get_bh(bh); 296 - J_ASSERT_JH(jh, !buffer_jbddirty(bh)); 297 325 BUFFER_TRACE(bh, "remove from checkpoint"); 298 326 __jbd2_journal_remove_checkpoint(jh); 299 327 spin_unlock(&journal->j_list_lock); 300 - jbd_unlock_bh_state(bh); 301 328 __brelse(bh); 302 329 } else { 303 330 /* ··· 310 341 J_ASSERT_BH(bh, !buffer_jwrite(bh)); 311 342 journal->j_chkpt_bhs[*batch_count] = bh; 312 343 __buffer_relink_io(jh); 313 - jbd_unlock_bh_state(bh); 314 344 transaction->t_chp_stats.cs_written++; 315 345 (*batch_count)++; 316 346 if (*batch_count == JBD2_NR_BATCH) { ··· 373 405 int retry = 0, err; 374 406 375 407 while (!retry && transaction->t_checkpoint_list) { 376 - struct buffer_head *bh; 377 - 378 408 jh = transaction->t_checkpoint_list; 379 - bh = jh2bh(jh); 380 - if (!jbd_trylock_bh_state(bh)) { 381 - jbd_sync_bh(journal, bh); 382 - retry = 1; 383 - break; 384 - } 385 409 retry = __process_buffer(journal, jh, &batch_count, 386 410 transaction); 387 411 if (retry < 0 && !result) ··· 489 529 do { 490 530 jh = next_jh; 491 531 next_jh = jh->b_cpnext; 492 - /* Use trylock because of the ranking */ 493 - if (jbd_trylock_bh_state(jh2bh(jh))) { 494 - ret = __try_to_free_cp_buf(jh); 495 - if (ret) { 496 - freed++; 497 - if (ret == 2) { 498 - *released = 1; 499 - return freed; 500 - } 532 + ret = __try_to_free_cp_buf(jh); 533 + if (ret) { 534 + freed++; 535 + if (ret == 2) { 536 + *released = 1; 537 + return freed; 501 538 } 502 539 } 503 540 /* ··· 577 620 * The function can free jh and bh. 578 621 * 579 622 * This function is called with j_list_lock held. 580 - * This function is called with jbd_lock_bh_state(jh2bh(jh)) 581 623 */ 582 - 583 624 int __jbd2_journal_remove_checkpoint(struct journal_head *jh) 584 625 { 585 626 struct transaction_chp_stats_s *stats;
+2
include/linux/journal-head.h
··· 66 66 * transaction (if there is one). Only applies to buffers on a 67 67 * transaction's data or metadata journaling list. 68 68 * [j_list_lock] [jbd_lock_bh_state()] 69 + * Either of these locks is enough for reading, both are needed for 70 + * changes. 69 71 */ 70 72 transaction_t *b_transaction; 71 73