Unverified Commit 1434918c authored by openeuler-ci-bot's avatar openeuler-ci-bot Committed by Gitee
Browse files

!1325 jbd2: fix several checkpoint

Merge Pull Request from: @ci-robot 
 
PR sync from: Zhihao Cheng <chengzhihao1@huawei.com>
https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/QARA5X5OQUKRFUIORG2YVB6YE3V5CGQB/ 
Zhang Yi (4):
  jbd2: remove journal_clean_one_cp_list()
  jbd2: fix a race when checking checkpoint buffer busy
  jbd2: remove __journal_try_to_free_buffer()
  jbd2: fix checkpoint cleanup performance regression

Zhihao Cheng (1):
  jbd2: Fix wrongly judgement for buffer head removing while doing
    checkpoint


-- 
2.31.1
 
 
Link:https://gitee.com/openeuler/kernel/pulls/1325

 

Reviewed-by: default avatarzhangyi (F) <yi.zhang@huawei.com>
Signed-off-by: default avatarJialin Zhang <zhangjialin11@huawei.com>
parents 2d388405 30b833d5
Loading
Loading
Loading
Loading
+74 −73
Original line number Diff line number Diff line
@@ -204,20 +204,6 @@ int jbd2_log_do_checkpoint(journal_t *journal)
		jh = transaction->t_checkpoint_list;
		bh = jh2bh(jh);

		/*
		 * The buffer may be writing back, or flushing out in the
		 * last couple of cycles, or re-adding into a new transaction,
		 * need to check it again until it's unlocked.
		 */
		if (buffer_locked(bh)) {
			get_bh(bh);
			spin_unlock(&journal->j_list_lock);
			wait_on_buffer(bh);
			/* the journal_head may have gone by now */
			BUFFER_TRACE(bh, "brelse");
			__brelse(bh);
			goto retry;
		}
		if (jh->b_transaction != NULL) {
			transaction_t *t = jh->b_transaction;
			tid_t tid = t->t_tid;
@@ -252,7 +238,22 @@ int jbd2_log_do_checkpoint(journal_t *journal)
			spin_lock(&journal->j_list_lock);
			goto restart;
		}
		if (!buffer_dirty(bh)) {
		if (!trylock_buffer(bh)) {
			/*
			 * The buffer is locked, it may be writing back, or
			 * flushing out in the last couple of cycles, or
			 * re-adding into a new transaction, need to check
			 * it again until it's unlocked.
			 */
			get_bh(bh);
			spin_unlock(&journal->j_list_lock);
			wait_on_buffer(bh);
			/* the journal_head may have gone by now */
			BUFFER_TRACE(bh, "brelse");
			__brelse(bh);
			goto retry;
		} else if (!buffer_dirty(bh)) {
			unlock_buffer(bh);
			BUFFER_TRACE(bh, "remove from checkpoint");
			/*
			 * If the transaction was released or the checkpoint
@@ -262,6 +263,7 @@ int jbd2_log_do_checkpoint(journal_t *journal)
			    !transaction->t_checkpoint_list)
				goto out;
		} else {
			unlock_buffer(bh);
			/*
			 * We are about to write the buffer, it could be
			 * raced by some other transaction shrink or buffer
@@ -347,50 +349,12 @@ int jbd2_cleanup_journal_tail(journal_t *journal)

/* Checkpoint list management */

/*
 * journal_clean_one_cp_list
 *
 * Find all the written-back checkpoint buffers in the given list and
 * release them. If 'destroy' is set, clean all buffers unconditionally.
 *
 * Called with j_list_lock held.
 * Returns 1 if we freed the transaction, 0 otherwise.
 */
static int journal_clean_one_cp_list(struct journal_head *jh, bool destroy)
{
	struct journal_head *last_jh;
	struct journal_head *next_jh = jh;

	if (!jh)
		return 0;

	last_jh = jh->b_cpprev;
	do {
		jh = next_jh;
		next_jh = jh->b_cpnext;

		if (!destroy && __cp_buffer_busy(jh))
			return 0;

		if (__jbd2_journal_remove_checkpoint(jh))
			return 1;
		/*
		 * 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())
			return 0;
	} while (jh != last_jh);

	return 0;
}
enum shrink_type {SHRINK_DESTROY, SHRINK_BUSY_STOP, SHRINK_BUSY_SKIP};

/*
 * journal_shrink_one_cp_list
 *
 * Find 'nr_to_scan' written-back checkpoint buffers in the given list
 * Find all the written-back checkpoint buffers in the given list
 * and try to release them. If the whole transaction is released, set
 * the 'released' parameter. Return the number of released checkpointed
 * buffers.
@@ -398,7 +362,7 @@ static int journal_clean_one_cp_list(struct journal_head *jh, bool destroy)
 * Called with j_list_lock held.
 */
static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
						unsigned long *nr_to_scan,
						enum shrink_type type,
						bool *released)
{
	struct journal_head *last_jh;
@@ -406,7 +370,8 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
	unsigned long nr_freed = 0;
	int ret;

	if (!jh || *nr_to_scan == 0)
	*released = false;
	if (!jh)
		return 0;

	last_jh = jh->b_cpprev;
@@ -414,12 +379,18 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
		jh = next_jh;
		next_jh = jh->b_cpnext;

		(*nr_to_scan)--;
		if (__cp_buffer_busy(jh))
		if (type == SHRINK_DESTROY) {
			ret = __jbd2_journal_remove_checkpoint(jh);
		} else {
			ret = jbd2_journal_try_remove_checkpoint(jh);
			if (ret < 0) {
				if (type == SHRINK_BUSY_SKIP)
					continue;
				break;
			}
		}

		nr_freed++;
		ret = __jbd2_journal_remove_checkpoint(jh);
		if (ret) {
			*released = true;
			break;
@@ -427,7 +398,7 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,

		if (need_resched())
			break;
	} while (jh != last_jh && *nr_to_scan);
	} while (jh != last_jh);

	return nr_freed;
}
@@ -445,11 +416,11 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
						  unsigned long *nr_to_scan)
{
	transaction_t *transaction, *last_transaction, *next_transaction;
	bool released;
	bool __maybe_unused released;
	tid_t first_tid = 0, last_tid = 0, next_tid = 0;
	tid_t tid = 0;
	unsigned long nr_freed = 0;
	unsigned long nr_scanned = *nr_to_scan;
	unsigned long freed;

again:
	spin_lock(&journal->j_list_lock);
@@ -478,10 +449,11 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
		transaction = next_transaction;
		next_transaction = transaction->t_cpnext;
		tid = transaction->t_tid;
		released = false;

		nr_freed += journal_shrink_one_cp_list(transaction->t_checkpoint_list,
						       nr_to_scan, &released);
		freed = journal_shrink_one_cp_list(transaction->t_checkpoint_list,
						   SHRINK_BUSY_SKIP, &released);
		nr_freed += freed;
		(*nr_to_scan) -= min(*nr_to_scan, freed);
		if (*nr_to_scan == 0)
			break;
		if (need_resched() || spin_needbreak(&journal->j_list_lock))
@@ -502,9 +474,8 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
	if (*nr_to_scan && next_tid)
		goto again;
out:
	nr_scanned -= *nr_to_scan;
	trace_jbd2_shrink_checkpoint_list(journal, first_tid, tid, last_tid,
					  nr_freed, nr_scanned, next_tid);
					  nr_freed, next_tid);

	return nr_freed;
}
@@ -520,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;
	int ret;
	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;
		ret = journal_clean_one_cp_list(transaction->t_checkpoint_list,
						destroy);
		journal_shrink_one_cp_list(transaction->t_checkpoint_list,
					   type, &released);
		/*
		 * This function only frees up some memory if possible so we
		 * dont have an obligation to finish processing. Bail out if
@@ -545,7 +518,7 @@ void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy)
		 * avoids pointless scanning of transactions which still
		 * weren't checkpointed.
		 */
		if (!ret)
		if (!released)
			return;
	} while (transaction != last_transaction);
}
@@ -655,6 +628,34 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
	return 1;
}

/*
 * Check the checkpoint buffer and try to remove it from the checkpoint
 * list if it's clean. Returns -EBUSY if it is not clean, returns 1 if
 * it frees the transaction, 0 otherwise.
 *
 * This function is called with j_list_lock held.
 */
int jbd2_journal_try_remove_checkpoint(struct journal_head *jh)
{
	struct buffer_head *bh = jh2bh(jh);

	if (!trylock_buffer(bh))
		return -EBUSY;
	if (buffer_dirty(bh)) {
		unlock_buffer(bh);
		return -EBUSY;
	}
	unlock_buffer(bh);

	/*
	 * Buffer is clean and the IO has finished (we held the buffer
	 * lock) so the checkpoint is done. We can safely remove the
	 * buffer from this transaction.
	 */
	JBUFFER_TRACE(jh, "remove from checkpoint list");
	return __jbd2_journal_remove_checkpoint(jh);
}

/*
 * journal_insert_checkpoint: put a committed buffer onto a checkpoint
 * list so that we know when it is safe to clean the transaction out of
+8 −32
Original line number Diff line number Diff line
@@ -1758,8 +1758,7 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
		 * Otherwise, if the buffer has been written to disk,
		 * it is safe to remove the checkpoint and drop it.
		 */
		if (!buffer_dirty(bh)) {
			__jbd2_journal_remove_checkpoint(jh);
		if (jbd2_journal_try_remove_checkpoint(jh) >= 0) {
			spin_unlock(&journal->j_list_lock);
			goto drop;
		}
@@ -2074,35 +2073,6 @@ void jbd2_journal_unfile_buffer(journal_t *journal, struct journal_head *jh)
	__brelse(bh);
}

/*
 * Called from jbd2_journal_try_to_free_buffers().
 *
 * Called under jh->b_state_lock
 */
static void
__journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh)
{
	struct journal_head *jh;

	jh = bh2jh(bh);

	if (buffer_locked(bh) || buffer_dirty(bh))
		goto out;

	if (jh->b_next_transaction != NULL || jh->b_transaction != NULL)
		goto out;

	spin_lock(&journal->j_list_lock);
	if (jh->b_cp_transaction != NULL) {
		/* written-back checkpointed metadata buffer */
		JBUFFER_TRACE(jh, "remove from checkpoint list");
		__jbd2_journal_remove_checkpoint(jh);
	}
	spin_unlock(&journal->j_list_lock);
out:
	return;
}

/**
 * jbd2_journal_try_to_free_buffers() - try to free page buffers.
 * @journal: journal for operation
@@ -2160,7 +2130,13 @@ int jbd2_journal_try_to_free_buffers(journal_t *journal, struct page *page)
			continue;

		spin_lock(&jh->b_state_lock);
		__journal_try_to_free_buffer(journal, bh);
		if (!jh->b_transaction && !jh->b_next_transaction) {
			spin_lock(&journal->j_list_lock);
			/* Remove written-back checkpointed metadata buffer */
			if (jh->b_cp_transaction != NULL)
				jbd2_journal_try_remove_checkpoint(jh);
			spin_unlock(&journal->j_list_lock);
		}
		spin_unlock(&jh->b_state_lock);
		jbd2_journal_put_journal_head(jh);
		if (buffer_jbd(bh))
+1 −0
Original line number Diff line number Diff line
@@ -1439,6 +1439,7 @@ extern void jbd2_journal_commit_transaction(journal_t *);
void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy);
unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal, unsigned long *nr_to_scan);
int __jbd2_journal_remove_checkpoint(struct journal_head *);
int jbd2_journal_try_remove_checkpoint(struct journal_head *jh);
void jbd2_journal_destroy_checkpoint(journal_t *journal);
void __jbd2_journal_insert_checkpoint(struct journal_head *, transaction_t *);

+4 −8
Original line number Diff line number Diff line
@@ -462,11 +462,9 @@ TRACE_EVENT(jbd2_shrink_scan_exit,
TRACE_EVENT(jbd2_shrink_checkpoint_list,

	TP_PROTO(journal_t *journal, tid_t first_tid, tid_t tid, tid_t last_tid,
		 unsigned long nr_freed, unsigned long nr_scanned,
		 tid_t next_tid),
		 unsigned long nr_freed, tid_t next_tid),

	TP_ARGS(journal, first_tid, tid, last_tid, nr_freed,
		nr_scanned, next_tid),
	TP_ARGS(journal, first_tid, tid, last_tid, nr_freed, next_tid),

	TP_STRUCT__entry(
		__field(dev_t, dev)
@@ -474,7 +472,6 @@ TRACE_EVENT(jbd2_shrink_checkpoint_list,
		__field(tid_t, tid)
		__field(tid_t, last_tid)
		__field(unsigned long, nr_freed)
		__field(unsigned long, nr_scanned)
		__field(tid_t, next_tid)
	),

@@ -484,15 +481,14 @@ TRACE_EVENT(jbd2_shrink_checkpoint_list,
		__entry->tid		= tid;
		__entry->last_tid	= last_tid;
		__entry->nr_freed	= nr_freed;
		__entry->nr_scanned	= nr_scanned;
		__entry->next_tid	= next_tid;
	),

	TP_printk("dev %d,%d shrink transaction %u-%u(%u) freed %lu "
		  "scanned %lu next transaction %u",
		  "next transaction %u",
		  MAJOR(__entry->dev), MINOR(__entry->dev),
		  __entry->first_tid, __entry->tid, __entry->last_tid,
		  __entry->nr_freed, __entry->nr_scanned, __entry->next_tid)
		  __entry->nr_freed, __entry->next_tid)
);

#endif /* _TRACE_JBD2_H */