linux-ext4 - Re: [PATCH v2 3/6] jbd2: remove journal_clean_one_cp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 6 Jun 2023 21:58:33 +0800
From:   Zhang Yi <yi.zhang@...weicloud.com>
To:     Jan Kara <jack@...e.cz>
Cc:     linux-ext4@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, yi.zhang@...wei.com, yukuai3@...wei.com,
        chengzhihao1@...wei.com
Subject: Re: [PATCH v2 3/6] jbd2: remove journal_clean_one_cp_list()

On 2023/6/6 15:46, Jan Kara wrote:
> On Tue 06-06-23 14:14:44, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@...wei.com>
>>
>> journal_clean_one_cp_list() and journal_shrink_one_cp_list() are almost
>> the same, so merge them into journal_shrink_one_cp_list(), remove the
>> nr_to_scan parameter, always scan and try to free the whole checkpoint
>> list.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
> 
> Looks good. Just one nit below:
> 
>> @@ -398,15 +358,14 @@ 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,
>> -						bool *released)
>> +						bool destroy, bool *released)
>>  {
>>  	struct journal_head *last_jh;
>>  	struct journal_head *next_jh = jh;
>>  	unsigned long nr_freed = 0;
>>  	int ret;
> 
> When changing this function, I think it will be more robust calling
> convention to unconditionally set *released = false at the beginning of
> this function. Then we can remove the initialization from
> __jbd2_journal_clean_checkpoint_list().
> 

Thanks for the review, will change it in my next iteration.

Yi.