Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 5 May 2023 15:13:28 +0200
From:   Jan Kara <jack@suse.cz>
To:     Zhang Yi <yi.zhang@huaweicloud.com>
Cc:     linux-ext4@vger.kernel.org, tytso@mit.edu,
        adilger.kernel@dilger.ca, jack@suse.cz, yi.zhang@huawei.com,
        yukuai3@huawei.com, chengzhihao1@huawei.com
Subject: Re: [PATCH v2 3/3] jbd2: remove released parameter in
 journal_shrink_one_cp_list()
Message-ID: <20230505131328.hh2x5v7w7yzmgksx@quack3>
References: <20230505123219.4135141-1-yi.zhang@huaweicloud.com>
 <20230505123219.4135141-3-yi.zhang@huaweicloud.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230505123219.4135141-3-yi.zhang@huaweicloud.com>
Precedence: bulk

On Fri 05-05-23 20:32:19, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> After t_checkpoint_io_list is gone, the 'released' parameter in
> journal_shrink_one_cp_list() becomes useless, just remove it.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/checkpoint.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 2b62154e9f1e..7b6320081f11 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -387,15 +387,13 @@ static int journal_clean_one_cp_list(struct journal_head *jh, bool destroy)
>   * journal_shrink_one_cp_list
>   *
>   * Find 'nr_to_scan' 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
> + * and try to release them. Return the number of released checkpointed
>   * buffers.
>   *
>   * Called with j_list_lock held.
>   */
>  static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
> -						unsigned long *nr_to_scan,
> -						bool *released)
> +						unsigned long *nr_to_scan)
>  {
>  	struct journal_head *last_jh;
>  	struct journal_head *next_jh = jh;
> @@ -416,10 +414,8 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
>  
>  		nr_freed++;
>  		ret = __jbd2_journal_remove_checkpoint(jh);
> -		if (ret) {
> -			*released = true;
> +		if (ret)
>  			break;
> -		}
>  
>  		if (need_resched())
>  			break;
> @@ -441,7 +437,6 @@ unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal,
>  						  unsigned long *nr_to_scan)
>  {
>  	transaction_t *transaction, *last_transaction, *next_transaction;
> -	bool released;
>  	tid_t first_tid = 0, last_tid = 0, next_tid = 0;
>  	tid_t tid = 0;
>  	unsigned long nr_freed = 0;
> @@ -474,10 +469,9 @@ 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);
> +						       nr_to_scan);
>  		if (*nr_to_scan == 0)
>  			break;
>  		if (need_resched() || spin_needbreak(&journal->j_list_lock))
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR