reiserfs b_count usage
PATCH 1/1 reiserfs b_count usage
Hi everyone,
This patch changes reiserfs to use get_bh and put_bh everywhere, instead of mucking around with b_count directly. In general, reiserfs uses brelse when doing the final release on a buffer, and it already has the buffer pinned during i/o because it wants to wait on the buffer later. In other words, we should not have the same races as in fs/buffer.c, but I made this patch to keep track with the changes there.
One hunk depends on the direct->indirect cleanup patch I posted last week (linus, I called this flushlist-3.diff in the batch I just sent you). I can supply one against pure pre7 if you would like.
Reiserfs already had a marco called get_bh, which was used to get a pointer to the last buffer head in a path in the tree. I've renamed that to get_last_bh.
Linus, please apply:
fs/reiserfs/do_balan.c +1 -1
@@ -1626,7 +1626,7 @@for (i =0 ; i <sizeof (tb->thrown)/sizeof (tb->thrown[0 ]); i ++)if (!tb->thrown[i]) { tb->thrown[i] = bh; -atomic_inc (&bh->b_count) ;/* decremented in free_thrown */ +get_bh (bh) ;/* free_thrown puts this */ return ; }reiserfs_warning ("store_thrown: too many thrown buffers\n" );
fs/reiserfs/fix_node.c +6 -6
@@ -1030,7 +1030,7 @@ left, left); }#endif -atomic_dec (&(left->b_count)); +put_bh (left) ;return 1 ; }
@@ -1123,7 +1123,7 @@ n_first_last_position =B_NR_ITEMS (p_s_parent);if ( n_position != n_first_last_position ) { *pp_s_com_father = p_s_parent; -atomic_inc (&((*pp_s_com_father)->b_count)); +get_bh (*pp_s_com_father) ;/*(*pp_s_com_father = p_s_parent)->b_count++;*/ break ; }
@@ -1228,8 +1228,8 @@/* Current node is not the first child of its parent. */ /*(p_s_curf = p_s_curcf = PATH_OFFSET_PBUFFER(p_s_path, n_path_offset - 1))->b_count += 2;*/ p_s_curf = p_s_curcf =PATH_OFFSET_PBUFFER (p_s_path, n_path_offset -1 ); -atomic_inc (&(p_s_curf->b_count)); -atomic_inc (&(p_s_curf->b_count)); +get_bh (p_s_curf) ; +get_bh (p_s_curf) ; p_s_tb->lkey[n_h] = n_position -1 ; }else {
@@ -1267,8 +1267,8 @@/* Current node is not the last child of its parent F[n_h]. */ /*(p_s_curf = p_s_curcf = PATH_OFFSET_PBUFFER(p_s_path, n_path_offset - 1))->b_count += 2;*/ p_s_curf = p_s_curcf =PATH_OFFSET_PBUFFER (p_s_path, n_path_offset -1 ); -atomic_inc (&(p_s_curf->b_count)); -atomic_inc (&(p_s_curf->b_count)); +get_bh (p_s_curf) ; +get_bh (p_s_curf) ; p_s_tb->rkey[n_h] = n_position; }
fs/reiserfs/inode.c +7 -7
@@ -274,7 +274,7 @@ }// - bh =get_bh (&path); + bh =get_last_bh (&path); ih =get_ih (&path);if (is_indirect_le_ih (ih)) {__u32 * ind_item = (__u32 *)B_I_PITEM (bh, ih);
@@ -369,7 +369,7 @@if (search_for_position_by_key (inode->i_sb, &key, &path) !=POSITION_FOUND )// we read something from tail, even if now we got IO_ERROR break ; - bh =get_bh (&path); + bh =get_last_bh (&path); ih =get_ih (&path); }while (1 );
@@ -577,7 +577,7 @@ goto failure; } - bh =get_bh (&path); + bh =get_last_bh (&path); ih =get_ih (&path); item =get_item (&path); pos_in_item = path.pos_in_item;
@@ -823,7 +823,7 @@pathrelse (&path) ; goto failure; } - bh =get_bh (&path); + bh =get_last_bh (&path); ih =get_ih (&path); item =get_item (&path); pos_in_item = path.pos_in_item;
@@ -1050,7 +1050,7 @@** FS might change. We have to detect that, and loop back to the ** search if the stat data item has moved */ - bh =get_bh (&path) ; + bh =get_last_bh (&path) ; ih =get_ih (&path) ;copy_item_head (&tmp_ih, ih); fs_gen =get_generation (inode->i_sb);
@@ -1679,7 +1679,7 @@ goto out ; } - bh =get_bh (&path) ; + bh =get_last_bh (&path) ; ih =get_ih (&path) ; item =get_item (&path) ; pos_in_item = path.pos_in_item ;
@@ -1761,7 +1761,7 @@for (i =0 ; i < nr ; i++) { bh = bhp[i] ;lock_buffer (bh) ; -atomic_inc (&bh->b_count) ;/* async end_io handler decs this */ +get_bh (bh) ;/* async end_io handler puts this */ set_buffer_async_io (bh) ;/* submit_bh doesn't care if the buffer is dirty, but nobody ** later on in the call chain will be cleaning it. So, we
fs/reiserfs/journal.c +13 -11
@@ -707,7 +707,7 @@ }ll_rw_block (WRITE ,1 , &tbh) ; count++ ; -atomic_dec (&(tbh->b_count)) ;/* once for our get_hash */ +put_bh (tbh) ;/* once for our get_hash */ } }
@@ -722,7 +722,7 @@if (!buffer_uptodate (tbh)) {reiserfs_panic (s,"journal-601, buffer write failed\n" ) ; } -atomic_dec (&(tbh->b_count)) ;/* once for our get_hash */ +put_bh (tbh) ;/* once for our get_hash */ bforget (tbh) ;/* once due to original getblk in do_journal_end */ atomic_dec (&(jl->j_commit_left)) ; }
@@ -869,9 +869,11 @@ }mark_buffer_uptodate (bh, uptodate) ;unlock_buffer (bh) ; +put_bh (bh) ; }static void submit_logged_buffer (struct buffer_head *bh) {lock_buffer (bh) ; +get_bh (bh) ; bh->b_end_io = reiserfs_end_buffer_io_sync ;mark_buffer_notjournal_new (bh) ;clear_bit (BH_Dirty, &bh->b_state) ;
@@ -967,13 +969,13 @@/* we do this to make sure nobody releases the buffer while ** we are working with it */ -atomic_inc (&(saved_bh->b_count)) ; +get_bh (saved_bh) ;if (buffer_journal_dirty (saved_bh)) { was_jwait =1 ;mark_buffer_notjournal_dirty (saved_bh) ; -/* brelse the inc from journal_mark_dirty */ -atomic_dec (&(saved_bh->b_count)) ; +/* undo the inc from journal_mark_dirty */ +put_bh (saved_bh) ; }if (can_dirty (cn)) { was_dirty =1 ;
@@ -1016,7 +1018,7 @@ }if (was_dirty) {/* we inc again because saved_bh gets decremented at free_cnode */ -atomic_inc (&(saved_bh->b_count)) ; +get_bh (saved_bh) ;set_bit (BLOCK_NEEDS_FLUSH , &cn->state) ;submit_logged_buffer (saved_bh) ; count++ ;
@@ -1029,7 +1031,7 @@ cn = cn->next ;if (saved_bh) {/* we incremented this to keep others from taking the buffer head away */ -atomic_dec (&(saved_bh->b_count)); +put_bh (saved_bh) ;if (atomic_read (&(saved_bh->b_count)) <0 ) {printk ("journal-945: saved_bh->b_count < 0" ) ; }
@@ -2176,7 +2178,7 @@ cn->jlist =NULL ;insert_journal_hash (SB_JOURNAL (p_s_sb)->j_hash_table, cn) ;if (!count_already_incd) { -atomic_inc (&(bh->b_count)) ; +get_bh (bh) ; } } cn->next =NULL ;
@@ -2248,7 +2250,7 @@if (!already_cleaned) {mark_buffer_notjournal_dirty (bh) ; -atomic_dec (&(bh->b_count)) ; +put_bh (bh) ;if (atomic_read (&(bh->b_count)) <0 ) {printk ("journal-1752: remove from trans, b_count < 0\n" ) ; }
@@ -2580,7 +2582,7 @@*/ mark_buffer_notjournal_dirty (cn->bh) ; cleaned =1 ; -atomic_dec (&(cn->bh->b_count)) ; +put_bh (cn->bh) ;if (atomic_read (&(cn->bh->b_count)) <0 ) {printk ("journal-2138: cn->bh->b_count < 0\n" ) ; }
@@ -2597,7 +2599,7 @@if (bh) {reiserfs_clean_and_file_buffer (bh) ; -atomic_dec (&(bh->b_count)) ;/* get_hash incs this */ +put_bh (bh) ;/* get_hash grabs the buffer */ if (atomic_read (&(bh->b_count)) <0 ) {printk ("journal-2165: bh->b_count < 0\n" ) ; }
fs/reiserfs/namei.c +1 -1
@@ -72,7 +72,7 @@// comment? maybe something like set de to point to what the path points to? static inline void set_de_item_location (struct reiserfs_dir_entry * de,struct path * path) { - de->de_bh =get_bh (path); + de->de_bh =get_last_bh (path); de->de_ih =get_ih (path); de->de_deh =B_I_DEH (de->de_bh, de->de_ih); de->de_item_num =PATH_LAST_POSITION (path);
fs/reiserfs/stree.c +5 -5
@@ -436,7 +436,7 @@ ) {if ( p_s_bh ) {if (atomic_read (&(p_s_bh->b_count)) ) { -atomic_dec (&(p_s_bh->b_count)); +put_bh (p_s_bh) ;return ; }reiserfs_panic (NULL ,"PAP-5070: decrement_bcount: trying to free free buffer %b" , p_s_bh);
@@ -1032,7 +1032,7 @@ }/* Cut one record from the directory item. */ - *cut_size = -(DEH_SIZE +entry_length (get_bh (path), le_ih,pos_in_item (path))); + *cut_size = -(DEH_SIZE +entry_length (get_last_bh (path), le_ih,pos_in_item (path)));return M_CUT ; }
@@ -1945,15 +1945,15 @@struct item_head * found_ih =get_ih (path);if (is_direct_le_ih (found_ih)) { -if (le_ih_k_offset (found_ih) +op_bytes_number (found_ih,get_bh (path)->b_size) != +if (le_ih_k_offset (found_ih) +op_bytes_number (found_ih,get_last_bh (path)->b_size) !=cpu_key_k_offset (p_s_key) || -op_bytes_number (found_ih,get_bh (path)->b_size) !=pos_in_item (path)) +op_bytes_number (found_ih,get_last_bh (path)->b_size) !=pos_in_item (path))reiserfs_panic (0 ,"PAP-5720: check_research_for_paste: " "found direct item %h or position (%d) does not match to key %K" , found_ih,pos_in_item (path), p_s_key); }if (is_indirect_le_ih (found_ih)) { -if (le_ih_k_offset (found_ih) +op_bytes_number (found_ih,get_bh (path)->b_size) !=cpu_key_k_offset (p_s_key) || +if (le_ih_k_offset (found_ih) +op_bytes_number (found_ih,get_last_bh (path)->b_size) !=cpu_key_k_offset (p_s_key) ||I_UNFM_NUM (found_ih) !=pos_in_item (path) ||get_ih_free_space (found_ih) !=0 ) reiserfs_panic (0, "PAP-5730: check_research_for_paste: "
fs/buffer.c +0 -11
@@ -71,17 +71,6 @@#define BH_ENTRY (list) list_entry((list), struct buffer_head, b_inode_buffers) -static inline void get_bh (struct buffer_head * bh) -{ -atomic_inc (&(bh)->b_count); -} - -static inline void put_bh (struct buffer_head *bh) -{ -smp_mb__before_atomic_dec (); -atomic_dec (&bh->b_count); -} -/* * Hash table gook.. */
include/linux/fs.h +11 -0
@@ -1069,6 +1069,17 @@#define BUF_PROTECTED 3/* Ramdisk persistent storage */ #define NR_LIST 4 +static inline void get_bh (struct buffer_head * bh) +{ +atomic_inc (&(bh)->b_count); +} + +static inline void put_bh (struct buffer_head *bh) +{ +smp_mb__before_atomic_dec (); +atomic_dec (&bh->b_count); +} +/* * This is called by bh->b_end_io() handlers when I/O has completed. */
include/linux/reiserfs_fs.h +1 -1
@@ -1102,7 +1106,7 @@#define PATH_H_PATH_OFFSET (p_s_path, n_h) ((p_s_path)->path_length - (n_h)) -#define get_bh (path) PATH_PLAST_BUFFER(path) +#define get_last_bh (path) PATH_PLAST_BUFFER(path)#define get_ih (path) PATH_PITEM_HEAD(path)#define get_item_pos (path) PATH_LAST_POSITION(path)#define get_item (path) ((void *)B_N_PITEM(PATH_PLAST_BUFFER(path), PATH_LAST_POSITION (path)))
fs/reiserfs/journal.c line 2605
Quoted 9 lines · Chris Mason
fs/reiserfs/journal.c +1
-1@@ -2597,7 +2599,7 @@if (bh) {reiserfs_clean_and_file_buffer (bh) ; -atomic_dec (&(bh->b_count)) ;/* get_hash incs this */ +put_bh (bh) ;/* get_hash grabs the buffer */ if (atomic_read (&(bh->b_count)) <0 ) {printk ("journal-2165: bh->b_count < 0\n" ) ; }
in mainline you aren't calling reiserfs_clean_and_file_buffer above, so it rejects.
Andrea
Quoted 13 lines · On Wednesday, July 18, 2001 05:53:42 PM +0200 Andrea Arcangeli…
fs/reiserfs/journal.c line 2605
Quoted 9 lines · Chris Mason
fs/reiserfs/journal.c +1
-1@@ -2597,7 +2599,7 @@if (bh) {reiserfs_clean_and_file_buffer (bh) ; -atomic_dec (&(bh->b_count)) ;/* get_hash incs this */ +put_bh (bh) ;/* get_hash grabs the buffer */ if (atomic_read (&(bh->b_count)) <0 ) {printk ("journal-2165: bh->b_count < 0\n" ) ; }in mainline you aren't calling
reiserfs_clean_and_file_bufferabove, so it rejects.
Correct, it goes cleanly on top of the direct->indirect cleanup patch that was sent to l-k last week, and sent to linus for inclusion. ac already has it, so it should apply there.
-chris