[PATCH] JBD: journal_try_to_free_buffers race fix

There is a race between transaction commit's attempt to free journal_heads
and journal_try_to_free_buffers' attempt.

Fix that by taking a ref against the journal_head in
journal_try_to_free_buffers().

diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 4889809093198a68f718a7ce96df45dc6ef28113..bc371d019bd28b9b9780f357f9b67910552e3906 100644 (file)
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -1679,6 +1679,23 @@ repeat:
        return bh->b_private;
 }
 
+/*
+ * Grab a ref against this buffer_head's journal_head.  If it ended up not
+ * having a journal_head, return NULL
+ */
+struct journal_head *journal_grab_journal_head(struct buffer_head *bh)
+{
+       struct journal_head *jh = NULL;
+
+       jbd_lock_bh_journal_head(bh);
+       if (buffer_jbd(bh)) {
+               jh = bh2jh(bh);
+               jh->b_jcount++;
+       }
+       jbd_unlock_bh_journal_head(bh);
+       return jh;
+}
+
 static void __journal_remove_journal_head(struct buffer_head *bh)
 {
        struct journal_head *jh = bh2jh(bh);

diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c
index 6c335edc00adff20fc2c23a856f99e8eedfb4f82..b08c36640ff5b1a814dd63983eccf2e1f3275634 100644 (file)
--- a/fs/jbd/transaction.c
+++ b/fs/jbd/transaction.c
@@ -1553,10 +1553,8 @@ void journal_unfile_buffer(journal_t *journal, struct journal_head *jh)
  * Called from journal_try_to_free_buffers().
  *
  * Called under jbd_lock_bh_state(bh)
- *
- * Returns non-zero iff we were able to free the journal_head.
  */
-static inline int
+static void
 __journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh)
 {
        struct journal_head *jh;
@@ -1589,10 +1587,8 @@ __journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh)
                }
        }
        spin_unlock(&journal->j_list_lock);
-       return !buffer_jbd(bh);
-
 out:
-       return 0;
+       return;
 }
 
 
@@ -1642,18 +1638,23 @@ int journal_try_to_free_buffers(journal_t *journal,
        head = page_buffers(page);
        bh = head;
        do {
-               jbd_lock_bh_state(bh);
+               struct journal_head *jh;
+
                /*
-                * We don't have to worry about the buffer being pulled off its
-                * journal_head in here, because __try_to_free_cp_buf runs
-                * under jbd_lock_bh_state()
+                * We take our own ref against the journal_head here to avoid
+                * having to add tons of locking around each instance of
+                * journal_remove_journal_head() and journal_put_journal_head().
                 */
-               if (buffer_jbd(bh) &&
-                               !__journal_try_to_free_buffer(journal, bh)) {
-                       jbd_unlock_bh_state(bh);
-                       goto busy;
-               }
+               jh = journal_grab_journal_head(bh);
+               if (!jh)
+                       continue;
+
+               jbd_lock_bh_state(bh);
+               __journal_try_to_free_buffer(journal, bh);
+               journal_put_journal_head(jh);
                jbd_unlock_bh_state(bh);
+               if (buffer_jbd(bh))
+                       goto busy;
        } while ((bh = bh->b_this_page) != head);
        ret = try_to_free_buffers(page);
 busy:

diff --git a/include/linux/jbd.h b/include/linux/jbd.h
index 567c835eb73eb81dd85455a04df960884e436e35..645e69eef43c7856d29a728141d6a9a648fca52a 100644 (file)
--- a/include/linux/jbd.h
+++ b/include/linux/jbd.h
@@ -943,10 +943,10 @@ extern int           journal_force_commit(journal_t *);
 /*
  * journal_head management
  */
-extern struct journal_head
-               *journal_add_journal_head(struct buffer_head *bh);
-extern void    journal_remove_journal_head(struct buffer_head *bh);
-extern void    journal_put_journal_head(struct journal_head *jh);
+struct journal_head *journal_add_journal_head(struct buffer_head *bh);
+struct journal_head *journal_grab_journal_head(struct buffer_head *bh);
+void journal_remove_journal_head(struct buffer_head *bh);
+void journal_put_journal_head(struct journal_head *jh);
 
 /*
  * handle management