--- /dev/null
+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 <alex@clusterfs.com>
+
+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-08 11:55:04.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);
+
+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-08 11:55:04.000000000 +0300
+@@ -229,6 +229,22 @@
+ jbd_debug (3, "JBD: commit phase 2\n");
+
+ /*
++ * First, drop modified flag: all accesses to the buffers
++ * will be tracked for a new trasaction only -bzzz
++ */
++ spin_lock(&journal->j_list_lock);
++ 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->b_modified = 0;
++ new_jh = new_jh->b_tnext;
++ } while (new_jh != jh);
++ }
++ spin_unlock(&journal->j_list_lock);
++
++ /*
+ * Now start flushing things to disk, in the order they appear
+ * on the transaction lists. Data blocks go first.
+ */
+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);
+ }
git://git.whamcloud.com / fs / lustre-release.git / commitdiff