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@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>
> 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.
