From: Alex Tomas <alex@clusterfs.com>
fix against credits leak in journal_release_buffer()
The idea is to charge a buffer in journal_dirty_metadata(), not in
journal_get_*_access()). Each buffer has flag call
journal_dirty_metadata() sets on the buffer.
Signed-off-by: Alex Tomas <alex@clusterfs.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/fs/ext3/balloc.c | 7 +---
25-akpm/fs/ext3/ialloc.c | 6 +--
25-akpm/fs/jbd/commit.c | 16 ++++++++++
25-akpm/fs/jbd/transaction.c | 56 ++++++++++++++---------------------
25-akpm/include/linux/ext3_jbd.h | 21 +++++--------
25-akpm/include/linux/jbd.h | 9 +----
25-akpm/include/linux/journal-head.h | 7 ++++
7 files changed, 63 insertions(+), 59 deletions(-)
diff -puN fs/ext3/balloc.c~jbd-journal-overflow-fix-2 fs/ext3/balloc.c
--- 25/fs/ext3/balloc.c~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.353019488 -0800
+++ 25-akpm/fs/ext3/balloc.c 2005-02-02 15:34:42.367017360 -0800
@@ -342,7 +342,7 @@ do_more:
*/
/* @@@ 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;
@@ -991,7 +991,6 @@ ext3_try_to_allocate_with_rsv(struct sup
unsigned long group_first_block;
int ret = 0;
int fatal;
- int credits = 0;
*errp = 0;
@@ -1001,7 +1000,7 @@ ext3_try_to_allocate_with_rsv(struct sup
* 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;
return -1;
@@ -1092,7 +1091,7 @@ out:
}
BUFFER_TRACE(bitmap_bh, "journal_release_buffer");
- ext3_journal_release_buffer(handle, bitmap_bh, credits);
+ ext3_journal_release_buffer(handle, bitmap_bh);
return ret;
}
diff -puN fs/ext3/ialloc.c~jbd-journal-overflow-fix-2 fs/ext3/ialloc.c
--- 25/fs/ext3/ialloc.c~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.354019336 -0800
+++ 25-akpm/fs/ext3/ialloc.c 2005-02-02 15:34:42.368017208 -0800
@@ -474,11 +474,9 @@ repeat_in_this_group:
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;
@@ -494,7 +492,7 @@ repeat_in_this_group:
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;
diff -puN fs/jbd/commit.c~jbd-journal-overflow-fix-2 fs/jbd/commit.c
--- 25/fs/jbd/commit.c~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.356019032 -0800
+++ 25-akpm/fs/jbd/commit.c 2005-02-02 15:34:42.369017056 -0800
@@ -229,6 +229,22 @@ void journal_commit_transaction(journal_
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.
*/
diff -puN fs/jbd/transaction.c~jbd-journal-overflow-fix-2 fs/jbd/transaction.c
--- 25/fs/jbd/transaction.c~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.358018728 -0800
+++ 25-akpm/fs/jbd/transaction.c 2005-02-02 15:34:42.371016752 -0800
@@ -522,7 +522,7 @@ static void jbd_unexpected_dirty_buffer(
*/
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;
@@ -604,11 +604,6 @@ repeat:
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;
}
@@ -688,10 +683,6 @@ repeat:
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
@@ -749,8 +740,7 @@ out:
* 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;
@@ -758,7 +748,7 @@ int journal_get_write_access(handle_t *h
/* 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;
}
@@ -814,9 +804,6 @@ int journal_get_create_access(handle_t *
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");
@@ -869,8 +856,7 @@ out:
*
* 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);
@@ -883,7 +869,7 @@ int journal_get_undo_access(handle_t *ha
* 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;
@@ -1111,6 +1097,17 @@ int journal_dirty_metadata(handle_t *han
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.
@@ -1161,24 +1158,11 @@ out:
* 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;
}
/**
@@ -1222,6 +1206,12 @@ int journal_forget (handle_t *handle, st
goto not_jbd;
}
+ /*
+ * 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);
diff -puN include/linux/ext3_jbd.h~jbd-journal-overflow-fix-2 include/linux/ext3_jbd.h
--- 25/include/linux/ext3_jbd.h~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.359018576 -0800
+++ 25-akpm/include/linux/ext3_jbd.h 2005-02-02 15:34:42.372016600 -0800
@@ -111,9 +111,9 @@ void ext3_journal_abort_handle(const cha
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;
@@ -121,19 +121,18 @@ __ext3_journal_get_undo_access(const cha
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 int
@@ -176,12 +175,10 @@ __ext3_journal_dirty_metadata(const char
}
-#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) \
diff -puN include/linux/jbd.h~jbd-journal-overflow-fix-2 include/linux/jbd.h
--- 25/include/linux/jbd.h~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.361018272 -0800
+++ 25-akpm/include/linux/jbd.h 2005-02-02 15:34:42.373016448 -0800
@@ -867,15 +867,12 @@ static inline handle_t *journal_current_
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 int journal_forget (handle_t *, struct buffer_head *);
extern void journal_sync_buffer (struct buffer_head *);
extern int journal_invalidatepage(journal_t *,
diff -puN include/linux/journal-head.h~jbd-journal-overflow-fix-2 include/linux/journal-head.h
--- 25/include/linux/journal-head.h~jbd-journal-overflow-fix-2 2005-02-02 15:34:42.363017968 -0800
+++ 25-akpm/include/linux/journal-head.h 2005-02-02 15:34:42.374016296 -0800
@@ -32,6 +32,13 @@ struct journal_head {
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()]
*/
_