fix against credits leak in journal_release_buffer() The idea is to charge a buffer at a time of modification (journal_dirty_metadata()), not at a time of access (journal_get_*_access()). Each buffer has flag first call journal_dirty_metadata() sets on the buffer. Signed-off-by: Alex Tomas Index: linux-2.6.7/include/linux/journal-head.h =================================================================== --- linux-2.6.7.orig/include/linux/journal-head.h 2003-06-24 18:05:26.000000000 +0400 +++ linux-2.6.7/include/linux/journal-head.h 2005-03-08 11:55:04.000000000 +0300 @@ -32,6 +32,13 @@ unsigned b_jlist; /* + * This flag signals the buffer has been modified by + * the currently running transaction + * [jbd_lock_bh_state()] + */ + unsigned b_modified; + + /* * Copy of the buffer data frozen for writing to the log. * [jbd_lock_bh_state()] */ Index: linux-2.6.7/include/linux/ext3_jbd.h =================================================================== --- linux-2.6.7.orig/include/linux/ext3_jbd.h 2004-08-26 17:12:16.000000000 +0400 +++ linux-2.6.7/include/linux/ext3_jbd.h 2005-03-08 11:55:04.000000000 +0300 @@ -113,9 +113,9 @@ static inline int __ext3_journal_get_undo_access(const char *where, handle_t *handle, - struct buffer_head *bh, int *credits) + struct buffer_head *bh) { - int err = journal_get_undo_access(handle, bh, credits); + int err = journal_get_undo_access(handle, bh); if (err) ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); return err; @@ -123,19 +123,18 @@ static inline int __ext3_journal_get_write_access(const char *where, handle_t *handle, - struct buffer_head *bh, int *credits) + struct buffer_head *bh) { - int err = journal_get_write_access(handle, bh, credits); + int err = journal_get_write_access(handle, bh); if (err) ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err); return err; } static inline void -ext3_journal_release_buffer(handle_t *handle, struct buffer_head *bh, - int credits) +ext3_journal_release_buffer(handle_t *handle, struct buffer_head *bh) { - journal_release_buffer(handle, bh, credits); + journal_release_buffer(handle, bh); } static inline void @@ -175,12 +174,10 @@ } -#define ext3_journal_get_undo_access(handle, bh, credits) \ - __ext3_journal_get_undo_access(__FUNCTION__, (handle), (bh), (credits)) +#define ext3_journal_get_undo_access(handle, bh) \ + __ext3_journal_get_undo_access(__FUNCTION__, (handle), (bh)) #define ext3_journal_get_write_access(handle, bh) \ - __ext3_journal_get_write_access(__FUNCTION__, (handle), (bh), NULL) -#define ext3_journal_get_write_access_credits(handle, bh, credits) \ - __ext3_journal_get_write_access(__FUNCTION__, (handle), (bh), (credits)) + __ext3_journal_get_write_access(__FUNCTION__, (handle), (bh)) #define ext3_journal_revoke(handle, blocknr, bh) \ __ext3_journal_revoke(__FUNCTION__, (handle), (blocknr), (bh)) #define ext3_journal_get_create_access(handle, bh) \ Index: linux-2.6.7/include/linux/jbd.h =================================================================== --- linux-2.6.7.orig/include/linux/jbd.h 2004-08-26 17:12:16.000000000 +0400 +++ linux-2.6.7/include/linux/jbd.h 2005-03-08 11:55:04.000000000 +0300 @@ -903,15 +903,12 @@ extern handle_t *journal_start(journal_t *, int nblocks); extern int journal_restart (handle_t *, int nblocks); extern int journal_extend (handle_t *, int nblocks); -extern int journal_get_write_access(handle_t *, struct buffer_head *, - int *credits); +extern int journal_get_write_access(handle_t *, struct buffer_head *); extern int journal_get_create_access (handle_t *, struct buffer_head *); -extern int journal_get_undo_access(handle_t *, struct buffer_head *, - int *credits); +extern int journal_get_undo_access(handle_t *, struct buffer_head *); extern int journal_dirty_data (handle_t *, struct buffer_head *); extern int journal_dirty_metadata (handle_t *, struct buffer_head *); -extern void journal_release_buffer (handle_t *, struct buffer_head *, - int credits); +extern void journal_release_buffer (handle_t *, struct buffer_head *); extern void journal_forget (handle_t *, struct buffer_head *); extern void journal_sync_buffer (struct buffer_head *); extern int journal_invalidatepage(journal_t *, Index: linux-2.6.7/fs/jbd/transaction.c =================================================================== --- linux-2.6.7.orig/fs/jbd/transaction.c 2004-08-26 17:12:40.000000000 +0400 +++ linux-2.6.7/fs/jbd/transaction.c 2005-03-11 00:04:54.000000000 +0300 @@ -524,7 +524,7 @@ */ static int do_get_write_access(handle_t *handle, struct journal_head *jh, - int force_copy, int *credits) + int force_copy) { struct buffer_head *bh; transaction_t *transaction; @@ -606,11 +606,6 @@ JBUFFER_TRACE(jh, "has frozen data"); J_ASSERT_JH(jh, jh->b_next_transaction == NULL); jh->b_next_transaction = transaction; - - J_ASSERT_JH(jh, handle->h_buffer_credits > 0); - handle->h_buffer_credits--; - if (credits) - (*credits)++; goto done; } @@ -689,10 +684,6 @@ jh->b_next_transaction = transaction; } - J_ASSERT(handle->h_buffer_credits > 0); - handle->h_buffer_credits--; - if (credits) - (*credits)++; /* * Finally, if the buffer is not journaled right now, we need to make @@ -750,8 +741,7 @@ * because we're write()ing a buffer which is also part of a shared mapping. */ -int journal_get_write_access(handle_t *handle, - struct buffer_head *bh, int *credits) +int journal_get_write_access(handle_t *handle, struct buffer_head *bh) { struct journal_head *jh = journal_add_journal_head(bh); int rc; @@ -759,7 +749,7 @@ /* We do not want to get caught playing with fields which the * log thread also manipulates. Make sure that the buffer * completes any outstanding IO before proceeding. */ - rc = do_get_write_access(handle, jh, 0, credits); + rc = do_get_write_access(handle, jh, 0); journal_put_journal_head(jh); return rc; } @@ -815,9 +805,6 @@ J_ASSERT_JH(jh, jh->b_next_transaction == NULL); J_ASSERT_JH(jh, buffer_locked(jh2bh(jh))); - J_ASSERT_JH(jh, handle->h_buffer_credits > 0); - handle->h_buffer_credits--; - if (jh->b_transaction == NULL) { jh->b_transaction = transaction; JBUFFER_TRACE(jh, "file as BJ_Reserved"); @@ -870,8 +857,7 @@ * * Returns error number or 0 on success. */ -int journal_get_undo_access(handle_t *handle, struct buffer_head *bh, - int *credits) +int journal_get_undo_access(handle_t *handle, struct buffer_head *bh) { int err; struct journal_head *jh = journal_add_journal_head(bh); @@ -884,7 +870,7 @@ * make sure that obtaining the committed_data is done * atomically wrt. completion of any outstanding commits. */ - err = do_get_write_access(handle, jh, 1, credits); + err = do_get_write_access(handle, jh, 1); if (err) goto out; @@ -1112,6 +1098,17 @@ jbd_lock_bh_state(bh); + if (jh->b_modified == 0) { + /* + * This buffer's got modified and becoming part + * of the transaction. This needs to be done + * once a transaction -bzzz + */ + jh->b_modified = 1; + J_ASSERT_JH(jh, handle->h_buffer_credits > 0); + handle->h_buffer_credits--; + } + /* * fastpath, to avoid expensive locking. If this buffer is already * on the running transaction's metadata list there is nothing to do. @@ -1162,24 +1159,11 @@ * journal_release_buffer: undo a get_write_access without any buffer * updates, if the update decided in the end that it didn't need access. * - * The caller passes in the number of credits which should be put back for - * this buffer (zero or one). - * - * We leave the buffer attached to t_reserved_list because even though this - * handle doesn't want it, some other concurrent handle may want to journal - * this buffer. If that handle is curently in between get_write_access() and - * journal_dirty_metadata() then it expects the buffer to be reserved. If - * we were to rip it off t_reserved_list here, the other handle will explode - * when journal_dirty_metadata is presented with a non-reserved buffer. - * - * If nobody really wants to journal this buffer then it will be thrown - * away at the start of commit. */ void -journal_release_buffer(handle_t *handle, struct buffer_head *bh, int credits) +journal_release_buffer(handle_t *handle, struct buffer_head *bh) { BUFFER_TRACE(bh, "entry"); - handle->h_buffer_credits += credits; } /** @@ -1214,6 +1198,12 @@ goto not_jbd; jh = bh2jh(bh); + /* + * The buffer's going from the transaction, we must drop + * all references -bzzz + */ + jh->b_modified = 0; + if (jh->b_transaction == handle->h_transaction) { J_ASSERT_JH(jh, !jh->b_frozen_data); @@ -2042,7 +2032,10 @@ __journal_unfile_buffer(jh); jh->b_transaction = jh->b_next_transaction; jh->b_next_transaction = NULL; - __journal_file_buffer(jh, jh->b_transaction, BJ_Metadata); + if (jh->b_modified == 1) + __journal_file_buffer(jh, jh->b_transaction, BJ_Metadata); + else + __journal_file_buffer(jh, jh->b_transaction, BJ_Reserved); J_ASSERT_JH(jh, jh->b_transaction->t_state == T_RUNNING); if (was_dirty) Index: linux-2.6.7/fs/jbd/commit.c =================================================================== --- linux-2.6.7.orig/fs/jbd/commit.c 2004-08-26 17:12:40.000000000 +0400 +++ linux-2.6.7/fs/jbd/commit.c 2005-03-11 00:46:54.000000000 +0300 @@ -204,6 +204,19 @@ } /* + * First, drop modified flag: all accesses to the buffers + * will be tracked for a new trasaction only -bzzz + */ + if (commit_transaction->t_buffers) { + new_jh = jh = commit_transaction->t_buffers->b_tnext; + do { + J_ASSERT_JH(new_jh, new_jh->b_modified == 1); + new_jh->b_modified = 0; + new_jh = new_jh->b_tnext; + } while (new_jh != jh); + } + + /* * Now try to drop any written-back buffers from the journal's * checkpoint lists. We do this *before* commit because it potentially * frees some memory Index: linux-2.6.7/fs/ext3/balloc.c =================================================================== --- linux-2.6.7.orig/fs/ext3/balloc.c 2004-08-26 17:11:16.000000000 +0400 +++ linux-2.6.7/fs/ext3/balloc.c 2005-03-08 11:55:04.000000000 +0300 @@ -171,7 +171,7 @@ */ /* @@@ check errors */ BUFFER_TRACE(bitmap_bh, "getting undo access"); - err = ext3_journal_get_undo_access(handle, bitmap_bh, NULL); + err = ext3_journal_get_undo_access(handle, bitmap_bh); if (err) goto error_return; @@ -411,7 +411,6 @@ { int i; int fatal; - int credits = 0; *errp = 0; @@ -421,7 +420,7 @@ * if the buffer is in BJ_Forget state in the committing transaction. */ BUFFER_TRACE(bitmap_bh, "get undo access for new block"); - fatal = ext3_journal_get_undo_access(handle, bitmap_bh, &credits); + fatal = ext3_journal_get_undo_access(handle, bitmap_bh); if (fatal) { *errp = fatal; goto fail; @@ -460,7 +459,7 @@ fail_access: BUFFER_TRACE(bitmap_bh, "journal_release_buffer"); - ext3_journal_release_buffer(handle, bitmap_bh, credits); + ext3_journal_release_buffer(handle, bitmap_bh); fail: return -1; } Index: linux-2.6.7/fs/ext3/ialloc.c =================================================================== --- linux-2.6.7.orig/fs/ext3/ialloc.c 2004-08-26 17:11:31.000000000 +0400 +++ linux-2.6.7/fs/ext3/ialloc.c 2005-03-08 11:55:04.000000000 +0300 @@ -475,11 +475,9 @@ ino = ext3_find_next_zero_bit((unsigned long *) bitmap_bh->b_data, EXT3_INODES_PER_GROUP(sb), ino); if (ino < EXT3_INODES_PER_GROUP(sb)) { - int credits = 0; BUFFER_TRACE(bitmap_bh, "get_write_access"); - err = ext3_journal_get_write_access_credits(handle, - bitmap_bh, &credits); + err = ext3_journal_get_write_access(handle, bitmap_bh); if (err) goto fail; @@ -495,7 +493,7 @@ goto got; } /* we lost it */ - journal_release_buffer(handle, bitmap_bh, credits); + journal_release_buffer(handle, bitmap_bh); if (++ino < EXT3_INODES_PER_GROUP(sb)) goto repeat_in_this_group; Index: linux-2.6.7/fs/ext3/xattr.c =================================================================== --- linux-2.6.7.orig/fs/ext3/xattr.c 2004-08-26 17:10:37.000000000 +0400 +++ linux-2.6.7/fs/ext3/xattr.c 2005-03-08 11:55:04.000000000 +0300 @@ -607,8 +607,7 @@ goto skip_get_write_access; /* ext3_journal_get_write_access() requires an unlocked bh, which complicates things here. */ - error = ext3_journal_get_write_access_credits(handle, bh, - &credits); + error = ext3_journal_get_write_access(handle, bh); if (error) goto cleanup; lock_buffer(bh); @@ -620,7 +619,7 @@ int offset; unlock_buffer(bh); - journal_release_buffer(handle, bh, credits); + journal_release_buffer(handle, bh); skip_get_write_access: ea_bdebug(bh, "cloning"); header = kmalloc(bh->b_size, GFP_KERNEL); @@ -764,8 +763,7 @@ error = -EDQUOT; if (DQUOT_ALLOC_BLOCK(inode, 1)) { unlock_buffer(new_bh); - journal_release_buffer(handle, new_bh, - credits); + journal_release_buffer(handle, new_bh); goto cleanup; } HDR(new_bh)->h_refcount = cpu_to_le32(1 + @@ -1057,8 +1055,7 @@ ext3_error(inode->i_sb, "ext3_xattr_cache_find", "inode %ld: block %ld read error", inode->i_ino, (unsigned long) ce->e_block); - } else if (ext3_journal_get_write_access_credits( - handle, bh, credits) == 0) { + } else if (ext3_journal_get_write_access(handle, bh) == 0) { /* ext3_journal_get_write_access() requires an unlocked * bh, which complicates things here. */ lock_buffer(bh); @@ -1074,7 +1071,7 @@ return bh; } unlock_buffer(bh); - journal_release_buffer(handle, bh, *credits); + journal_release_buffer(handle, bh); *credits = 0; brelse(bh); }