jbd2: avoid pointless scanning of checkpoint lists

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@linux.intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

1 files changed, 18 insertions, 14 deletions

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 1fbf59938cc0..3ab4c5ee12ce 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -420,7 +420,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)
  */
@@ -440,12 +439,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
@@ -465,7 +464,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)
  */
@@ -473,7 +471,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;
@@ -485,17 +484,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
@@ -503,11 +506,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;
 }
 
 /*