Commit cc97f1a7 authored by Jan Kara's avatar Jan Kara Committed by Theodore Ts'o

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: default avatarYuanhan Liu <yuanhan.liu@linux.intel.com>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 84474976
...@@ -420,7 +420,6 @@ int jbd2_cleanup_journal_tail(journal_t *journal) ...@@ -420,7 +420,6 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
* Find all the written-back checkpoint buffers in the given list and * Find all the written-back checkpoint buffers in the given list and
* release them. * release them.
* *
* Called with the journal locked.
* Called with j_list_lock held. * Called with j_list_lock held.
* Returns number of buffers reaped (for debug) * Returns number of buffers reaped (for debug)
*/ */
...@@ -440,12 +439,12 @@ static int journal_clean_one_cp_list(struct journal_head *jh, int *released) ...@@ -440,12 +439,12 @@ static int journal_clean_one_cp_list(struct journal_head *jh, int *released)
jh = next_jh; jh = next_jh;
next_jh = jh->b_cpnext; next_jh = jh->b_cpnext;
ret = __try_to_free_cp_buf(jh); ret = __try_to_free_cp_buf(jh);
if (ret) { if (!ret)
freed++; return freed;
if (ret == 2) { freed++;
*released = 1; if (ret == 2) {
return freed; *released = 1;
} return freed;
} }
/* /*
* This function only frees up some memory * This function only frees up some memory
...@@ -465,7 +464,6 @@ static int journal_clean_one_cp_list(struct journal_head *jh, int *released) ...@@ -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. * Find all the written-back checkpoint buffers in the journal and release them.
* *
* Called with the journal locked.
* Called with j_list_lock held. * Called with j_list_lock held.
* Returns number of buffers reaped (for debug) * Returns number of buffers reaped (for debug)
*/ */
...@@ -473,7 +471,8 @@ static int journal_clean_one_cp_list(struct journal_head *jh, int *released) ...@@ -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) int __jbd2_journal_clean_checkpoint_list(journal_t *journal)
{ {
transaction_t *transaction, *last_transaction, *next_transaction; transaction_t *transaction, *last_transaction, *next_transaction;
int ret = 0; int ret;
int freed = 0;
int released; int released;
transaction = journal->j_checkpoint_transactions; transaction = journal->j_checkpoint_transactions;
...@@ -485,17 +484,21 @@ int __jbd2_journal_clean_checkpoint_list(journal_t *journal) ...@@ -485,17 +484,21 @@ int __jbd2_journal_clean_checkpoint_list(journal_t *journal)
do { do {
transaction = next_transaction; transaction = next_transaction;
next_transaction = transaction->t_cpnext; next_transaction = transaction->t_cpnext;
ret += journal_clean_one_cp_list(transaction-> ret = journal_clean_one_cp_list(transaction->
t_checkpoint_list, &released); t_checkpoint_list, &released);
/* /*
* This function only frees up some memory if possible so we * This function only frees up some memory if possible so we
* dont have an obligation to finish processing. Bail out if * dont have an obligation to finish processing. Bail out if
* preemption requested: * preemption requested:
*/ */
if (need_resched()) if (need_resched()) {
freed += ret;
goto out; goto out;
if (released) }
if (released) {
freed += ret;
continue; continue;
}
/* /*
* It is essential that we are as careful as in the case of * It is essential that we are as careful as in the case of
* t_checkpoint_list with removing the buffer from the list as * t_checkpoint_list with removing the buffer from the list as
...@@ -503,11 +506,12 @@ int __jbd2_journal_clean_checkpoint_list(journal_t *journal) ...@@ -503,11 +506,12 @@ int __jbd2_journal_clean_checkpoint_list(journal_t *journal)
*/ */
ret += journal_clean_one_cp_list(transaction-> ret += journal_clean_one_cp_list(transaction->
t_checkpoint_io_list, &released); t_checkpoint_io_list, &released);
if (need_resched()) freed += ret;
if (need_resched() || !ret)
goto out; goto out;
} while (transaction != last_transaction); } while (transaction != last_transaction);
out: out:
return ret; return freed;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment