2023-05-05 12:39:52

by Zhang Yi

[permalink] [raw]
Subject: [PATCH v2 2/3] jbd2: remove t_checkpoint_io_list

From: Zhang Yi <[email protected]>

Since t_checkpoint_io_list was stop using in jbd2_log_do_checkpoint()
now, it's time to remove the whole t_checkpoint_io_list logic.

Signed-off-by: Zhang Yi <[email protected]>
---
fs/jbd2/checkpoint.c | 42 ++----------------------------------------
fs/jbd2/commit.c | 3 +--
include/linux/jbd2.h | 6 ------
3 files changed, 3 insertions(+), 48 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index ae1ebfb8bc86..2b62154e9f1e 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -27,7 +27,7 @@
*
* Called with j_list_lock held.
*/
-static inline void __buffer_unlink_first(struct journal_head *jh)
+static inline void __buffer_unlink(struct journal_head *jh)
{
transaction_t *transaction = jh->b_cp_transaction;

@@ -40,23 +40,6 @@ static inline void __buffer_unlink_first(struct journal_head *jh)
}
}

-/*
- * Unlink a buffer from a transaction checkpoint(io) list.
- *
- * Called with j_list_lock held.
- */
-static inline void __buffer_unlink(struct journal_head *jh)
-{
- transaction_t *transaction = jh->b_cp_transaction;
-
- __buffer_unlink_first(jh);
- if (transaction->t_checkpoint_io_list == jh) {
- transaction->t_checkpoint_io_list = jh->b_cpnext;
- if (transaction->t_checkpoint_io_list == jh)
- transaction->t_checkpoint_io_list = NULL;
- }
-}
-
/*
* Check a checkpoint buffer could be release or not.
*
@@ -499,15 +482,6 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
break;
if (need_resched() || spin_needbreak(&journal->j_list_lock))
break;
- if (released)
- continue;
-
- nr_freed += journal_shrink_one_cp_list(transaction->t_checkpoint_io_list,
- nr_to_scan, &released);
- if (*nr_to_scan == 0)
- break;
- if (need_resched() || spin_needbreak(&journal->j_list_lock))
- break;
} while (transaction != last_transaction);

if (transaction != last_transaction) {
@@ -562,17 +536,6 @@ void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy)
*/
if (need_resched())
return;
- if (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
- * we can possibly see not yet submitted buffers on io_list
- */
- ret = journal_clean_one_cp_list(transaction->
- t_checkpoint_io_list, destroy);
- if (need_resched())
- return;
/*
* Stop scanning if we couldn't free the transaction. This
* avoids pointless scanning of transactions which still
@@ -657,7 +620,7 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
jbd2_journal_put_journal_head(jh);

/* Is this transaction empty? */
- if (transaction->t_checkpoint_list || transaction->t_checkpoint_io_list)
+ if (transaction->t_checkpoint_list)
return 0;

/*
@@ -749,7 +712,6 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
J_ASSERT(transaction->t_forget == NULL);
J_ASSERT(transaction->t_shadow_list == NULL);
J_ASSERT(transaction->t_checkpoint_list == NULL);
- J_ASSERT(transaction->t_checkpoint_io_list == NULL);
J_ASSERT(atomic_read(&transaction->t_updates) == 0);
J_ASSERT(journal->j_committing_transaction != transaction);
J_ASSERT(journal->j_running_transaction != transaction);
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index b33155dd7001..1073259902a6 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -1141,8 +1141,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
spin_lock(&journal->j_list_lock);
commit_transaction->t_state = T_FINISHED;
/* Check if the transaction can be dropped now that we are finished */
- if (commit_transaction->t_checkpoint_list == NULL &&
- commit_transaction->t_checkpoint_io_list == NULL) {
+ if (commit_transaction->t_checkpoint_list == NULL) {
__jbd2_journal_drop_transaction(journal, commit_transaction);
jbd2_journal_free_transaction(commit_transaction);
}
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index f619bae1dcc5..91a2cf4bc575 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -622,12 +622,6 @@ struct transaction_s
*/
struct journal_head *t_checkpoint_list;

- /*
- * Doubly-linked circular list of all buffers submitted for IO while
- * checkpointing. [j_list_lock]
- */
- struct journal_head *t_checkpoint_io_list;
-
/*
* Doubly-linked circular list of metadata buffers being
* shadowed by log IO. The IO buffers on the iobuf list and
--
2.31.1


2023-05-05 13:17:02

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] jbd2: remove t_checkpoint_io_list

On Fri 05-05-23 20:32:18, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> Since t_checkpoint_io_list was stop using in jbd2_log_do_checkpoint()
> now, it's time to remove the whole t_checkpoint_io_list logic.
>
> Signed-off-by: Zhang Yi <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/jbd2/checkpoint.c | 42 ++----------------------------------------
> fs/jbd2/commit.c | 3 +--
> include/linux/jbd2.h | 6 ------
> 3 files changed, 3 insertions(+), 48 deletions(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index ae1ebfb8bc86..2b62154e9f1e 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -27,7 +27,7 @@
> *
> * Called with j_list_lock held.
> */
> -static inline void __buffer_unlink_first(struct journal_head *jh)
> +static inline void __buffer_unlink(struct journal_head *jh)
> {
> transaction_t *transaction = jh->b_cp_transaction;
>
> @@ -40,23 +40,6 @@ static inline void __buffer_unlink_first(struct journal_head *jh)
> }
> }
>
> -/*
> - * Unlink a buffer from a transaction checkpoint(io) list.
> - *
> - * Called with j_list_lock held.
> - */
> -static inline void __buffer_unlink(struct journal_head *jh)
> -{
> - transaction_t *transaction = jh->b_cp_transaction;
> -
> - __buffer_unlink_first(jh);
> - if (transaction->t_checkpoint_io_list == jh) {
> - transaction->t_checkpoint_io_list = jh->b_cpnext;
> - if (transaction->t_checkpoint_io_list == jh)
> - transaction->t_checkpoint_io_list = NULL;
> - }
> -}
> -
> /*
> * Check a checkpoint buffer could be release or not.
> *
> @@ -499,15 +482,6 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
> break;
> if (need_resched() || spin_needbreak(&journal->j_list_lock))
> break;
> - if (released)
> - continue;
> -
> - nr_freed += journal_shrink_one_cp_list(transaction->t_checkpoint_io_list,
> - nr_to_scan, &released);
> - if (*nr_to_scan == 0)
> - break;
> - if (need_resched() || spin_needbreak(&journal->j_list_lock))
> - break;
> } while (transaction != last_transaction);
>
> if (transaction != last_transaction) {
> @@ -562,17 +536,6 @@ void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy)
> */
> if (need_resched())
> return;
> - if (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
> - * we can possibly see not yet submitted buffers on io_list
> - */
> - ret = journal_clean_one_cp_list(transaction->
> - t_checkpoint_io_list, destroy);
> - if (need_resched())
> - return;
> /*
> * Stop scanning if we couldn't free the transaction. This
> * avoids pointless scanning of transactions which still
> @@ -657,7 +620,7 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
> jbd2_journal_put_journal_head(jh);
>
> /* Is this transaction empty? */
> - if (transaction->t_checkpoint_list || transaction->t_checkpoint_io_list)
> + if (transaction->t_checkpoint_list)
> return 0;
>
> /*
> @@ -749,7 +712,6 @@ void __jbd2_journal_drop_transaction(journal_t *journal, transaction_t *transact
> J_ASSERT(transaction->t_forget == NULL);
> J_ASSERT(transaction->t_shadow_list == NULL);
> J_ASSERT(transaction->t_checkpoint_list == NULL);
> - J_ASSERT(transaction->t_checkpoint_io_list == NULL);
> J_ASSERT(atomic_read(&transaction->t_updates) == 0);
> J_ASSERT(journal->j_committing_transaction != transaction);
> J_ASSERT(journal->j_running_transaction != transaction);
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index b33155dd7001..1073259902a6 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -1141,8 +1141,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> spin_lock(&journal->j_list_lock);
> commit_transaction->t_state = T_FINISHED;
> /* Check if the transaction can be dropped now that we are finished */
> - if (commit_transaction->t_checkpoint_list == NULL &&
> - commit_transaction->t_checkpoint_io_list == NULL) {
> + if (commit_transaction->t_checkpoint_list == NULL) {
> __jbd2_journal_drop_transaction(journal, commit_transaction);
> jbd2_journal_free_transaction(commit_transaction);
> }
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index f619bae1dcc5..91a2cf4bc575 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -622,12 +622,6 @@ struct transaction_s
> */
> struct journal_head *t_checkpoint_list;
>
> - /*
> - * Doubly-linked circular list of all buffers submitted for IO while
> - * checkpointing. [j_list_lock]
> - */
> - struct journal_head *t_checkpoint_io_list;
> -
> /*
> * Doubly-linked circular list of metadata buffers being
> * shadowed by log IO. The IO buffers on the iobuf list and
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR