xfs
[Top] [All Lists]

[PATCH 1/2] jbd2: Avoid pointless scanning of checkpoint lists

To: linux-fsdevel@xxxxxxxxxxxxxxx
Subject: [PATCH 1/2] jbd2: Avoid pointless scanning of checkpoint lists
From: Jan Kara <jack@xxxxxxx>
Date: Fri, 10 Oct 2014 16:23:23 +0200
Cc: linux-ext4@xxxxxxxxxxxxxxx, Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, cluster-devel@xxxxxxxxxx, Steven Whitehouse <swhiteho@xxxxxxxxxx>, Mark Fasheh <mfasheh@xxxxxxxx>, Joel Becker <jlbec@xxxxxxxxxxxx>, ocfs2-devel@xxxxxxxxxxxxxx, reiserfs-devel@xxxxxxxxxxxxxxx, Jeff Mahoney <jeffm@xxxxxxx>, Dave Kleikamp <shaggy@xxxxxxxxxx>, jfs-discussion@xxxxxxxxxxxxxxxxxxxxx, tytso@xxxxxxx, viro@xxxxxxxxxxxxxxxxxx, Jan Kara <jack@xxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1412951028-4085-1-git-send-email-jack@xxxxxxx>
References: <1412951028-4085-1-git-send-email-jack@xxxxxxx>
Yuanhan has reported that when he is running fsync(2) heavy workload
creating new files over ramdisk, significant amount of time is spent in
__jbd2_journal_clean_checkpoint_list() trying to clean old transactions
(but they cannot be cleaned up because flusher hasn't yet checkpointed
those buffers). The workload can be generated by:
  fs_mark -d /fs/ram0/1 -D 2 -N 2560 -n 1000000 -L 1 -S 1 -s 4096

Reduce the amount of scanning by stopping to scan the transaction list
once we find a transaction that cannot be checkpointed. Note that this
way of cleaning is still enough to keep freeing space in the journal
after fully checkpointed transactions.

Reported-and-tested-by: Yuanhan Liu <yuanhan.liu@xxxxxxxxxxxxxxx>
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
 fs/jbd2/checkpoint.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 7f34f4716165..e39b2d0e1079 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -478,7 +478,6 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
  * Find all the written-back checkpoint buffers in the given list and
  * release them.
  *
- * Called with the journal locked.
  * Called with j_list_lock held.
  * Returns number of buffers reaped (for debug)
  */
@@ -498,12 +497,12 @@ static int journal_clean_one_cp_list(struct journal_head 
*jh, int *released)
                jh = next_jh;
                next_jh = jh->b_cpnext;
                ret = __try_to_free_cp_buf(jh);
-               if (ret) {
-                       freed++;
-                       if (ret == 2) {
-                               *released = 1;
-                               return freed;
-                       }
+               if (!ret)
+                       return freed;
+               freed++;
+               if (ret == 2) {
+                       *released = 1;
+                       return freed;
                }
                /*
                 * This function only frees up some memory
@@ -523,7 +522,6 @@ static int journal_clean_one_cp_list(struct journal_head 
*jh, int *released)
  *
  * Find all the written-back checkpoint buffers in the journal and release 
them.
  *
- * Called with the journal locked.
  * Called with j_list_lock held.
  * Returns number of buffers reaped (for debug)
  */
@@ -531,7 +529,8 @@ static int journal_clean_one_cp_list(struct journal_head 
*jh, int *released)
 int __jbd2_journal_clean_checkpoint_list(journal_t *journal)
 {
        transaction_t *transaction, *last_transaction, *next_transaction;
-       int ret = 0;
+       int ret;
+       int freed = 0;
        int released;
 
        transaction = journal->j_checkpoint_transactions;
@@ -543,17 +542,21 @@ int __jbd2_journal_clean_checkpoint_list(journal_t 
*journal)
        do {
                transaction = next_transaction;
                next_transaction = transaction->t_cpnext;
-               ret += journal_clean_one_cp_list(transaction->
+               ret = journal_clean_one_cp_list(transaction->
                                t_checkpoint_list, &released);
                /*
                 * This function only frees up some memory if possible so we
                 * dont have an obligation to finish processing. Bail out if
                 * preemption requested:
                 */
-               if (need_resched())
+               if (need_resched()) {
+                       freed += ret;
                        goto out;
-               if (released)
+               }
+               if (released) {
+                       freed += ret;
                        continue;
+               }
                /*
                 * It is essential that we are as careful as in the case of
                 * t_checkpoint_list with removing the buffer from the list as
@@ -561,11 +564,12 @@ int __jbd2_journal_clean_checkpoint_list(journal_t 
*journal)
                 */
                ret += journal_clean_one_cp_list(transaction->
                                t_checkpoint_io_list, &released);
-               if (need_resched())
+               freed += ret;
+               if (need_resched() || !ret)
                        goto out;
        } while (transaction != last_transaction);
 out:
-       return ret;
+       return freed;
 }
 
 /*
-- 
1.8.1.4

<Prev in Thread] Current Thread [Next in Thread>