Commit 3a1a769a authored by Zhang Yi's avatar Zhang Yi Committed by openeuler-sync-bot
Browse files

jbd2: fix checkpoint cleanup performance regression 

hulk inclusion
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I7IO1D


CVE: NA

--------------------------------

journal_clean_one_cp_list() has been merged into
journal_shrink_one_cp_list(), but do chekpoint buffer cleanup from the
committing process is just a best effort, it should stop scan once it
meet a busy buffer, or else it will cause a lot of invalid buffer scan
and checks. We catch a performance regression when doing fs_mark tests
below.

Test cmd:
 ./fs_mark  -d  scratch  -s  1024  -n  10000  -t  1  -D  100  -N  100

Before merging checkpoint buffer cleanup:
 FSUse%        Count         Size    Files/sec     App Overhead
     95        10000         1024       8304.9            49033

After merging checkpoint buffer cleanup:
 FSUse%        Count         Size    Files/sec     App Overhead
     95        10000         1024       7649.0            50012
 FSUse%        Count         Size    Files/sec     App Overhead
     95        10000         1024       2107.1            50871

After merging checkpoint buffer cleanup, the total loop count in
journal_shrink_one_cp_list() could be up to 6,261,600+ (50,000+ ~
100,000+ in general), most of them are invalid. This patch fix it
through passing 'shrink_type' into journal_shrink_one_cp_list() and add
a new 'SHRINK_BUSY_STOP' to indicate it should stop once meet a busy
buffer. After fix, the loop count descending back to 10,000+.

After this fix:
 FSUse%        Count         Size    Files/sec     App Overhead
     95        10000         1024       8558.4            49109

Signed-off-by: default avatarZhang Yi <yi.zhang@huawei.com>
Signed-off-by: default avatarZhihao Cheng <chengzhihao1@huawei.com>
(cherry picked from commit 30b833d5)
parent 7b5c67e4

fs/jbd2/checkpoint.c

+14 −6
Original line number Diff line number Diff line
@@ -349,6 +349,8 @@ int jbd2_cleanup_journal_tail(journal_t *journal) 


/* Checkpoint list management */



enum shrink_type {SHRINK_DESTROY, SHRINK_BUSY_STOP, SHRINK_BUSY_SKIP};



/*

 * journal_shrink_one_cp_list

 *
@@ -360,7 +362,8 @@ int jbd2_cleanup_journal_tail(journal_t *journal) 
 * Called with j_list_lock held.

 */

static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,

						bool destroy, bool *released)

						enum shrink_type type,

						bool *released)

{

	struct journal_head *last_jh;

	struct journal_head *next_jh = jh;
@@ -376,12 +379,15 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh, 
		jh = next_jh;

		next_jh = jh->b_cpnext;



		if (destroy) {

		if (type == SHRINK_DESTROY) {

			ret = __jbd2_journal_remove_checkpoint(jh);

		} else {

			ret = jbd2_journal_try_remove_checkpoint(jh);

			if (ret < 0)

			if (ret < 0) {

				if (type == SHRINK_BUSY_SKIP)

					continue;

				break;

			}

		}



		nr_freed++;
@@ -445,7 +451,7 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal, 
		tid = transaction->t_tid;



		freed = journal_shrink_one_cp_list(transaction->t_checkpoint_list,

						   false, &released);

						   SHRINK_BUSY_SKIP, &released);

		nr_freed += freed;

		(*nr_to_scan) -= min(*nr_to_scan, freed);

		if (*nr_to_scan == 0)
@@ -485,19 +491,21 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal, 
void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy)

{

	transaction_t *transaction, *last_transaction, *next_transaction;

	enum shrink_type type;

	bool released;



	transaction = journal->j_checkpoint_transactions;

	if (!transaction)

		return;



	type = destroy ? SHRINK_DESTROY : SHRINK_BUSY_STOP;

	last_transaction = transaction->t_cpprev;

	next_transaction = transaction;

	do {

		transaction = next_transaction;

		next_transaction = transaction->t_cpnext;

		journal_shrink_one_cp_list(transaction->t_checkpoint_list,

					   destroy, &released);

					   type, &released);

		/*

		 * This function only frees up some memory if possible so we

		 * dont have an obligation to finish processing. Bail out if