linux-kernel - Re: [PATCH] f2fs: continuous counting for 'issued' in __issue_discard_cmd

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]

Message-ID: <547e58b1-942e-8beb-29a2-3920184c46a0@gmail.com>
Date:   Sun, 11 Dec 2022 20:08:39 +0800
From:   Yuwei Guan <ssawgyw@...il.com>
To:     Chao Yu <chao@...nel.org>, jaegeuk@...nel.org
Cc:     linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, Yuwei.Guan@...krlife.com
Subject: Re: [PATCH] f2fs: continuous counting for 'issued' in
 __issue_discard_cmd_orderly()



在 2022/12/11 10:36, Chao Yu 写道:
> On 2022/11/22 0:11, Yuwei Guan wrote:
>> As the 'dcc->discard_granularity' and 'dcc->max_ordered_discard' can 
>> be set
>> at the user space, and if the 'dcc->max_ordered_discard' is set larger 
>> than
>> 'dcc->discard_granularity' in DPOLICY_BG mode, or it's a volume device,
>> discard_granularity can be tuned to 1 in f2fs_tuning_parameters(),
>> it will may send more requests than the number of 'dpolicy->max_requests'
>> in issue_discard_thread().
>>
>> This patch will fix the issue.
>>
>> Signed-off-by: Yuwei Guan <Yuwei.Guan@...krlife.com>
>> ---
>>   fs/f2fs/segment.c | 17 ++++++++---------
>>   1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 8b0b76550578..b0157bf392fb 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1379,8 +1379,8 @@ static int __queue_discard_cmd(struct 
>> f2fs_sb_info *sbi,
>>       return 0;
>>   }
>> -static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info 
>> *sbi,
>> -                    struct discard_policy *dpolicy)
>> +static int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
>> +                    struct discard_policy *dpolicy, int *issued)
>>   {
>>       struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
>>       struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
>> @@ -1388,7 +1388,6 @@ static unsigned int 
>> __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
>>       struct discard_cmd *dc;
>>       struct blk_plug plug;
>>       unsigned int pos = dcc->next_pos;
>> -    unsigned int issued = 0;
>>       bool io_interrupted = false;
>>       mutex_lock(&dcc->cmd_lock);
>> @@ -1415,9 +1414,9 @@ static unsigned int 
>> __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
>>           }
>>           dcc->next_pos = dc->lstart + dc->len;
>> -        err = __submit_discard_cmd(sbi, dpolicy, dc, &issued);
>> +        err = __submit_discard_cmd(sbi, dpolicy, dc, issued);
>> -        if (issued >= dpolicy->max_requests)
>> +        if (*issued >= dpolicy->max_requests)
>>               break;
>>   next:
>>           node = rb_next(&dc->rb_node);
>> @@ -1433,10 +1432,10 @@ static unsigned int 
>> __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
>>       mutex_unlock(&dcc->cmd_lock);
>> -    if (!issued && io_interrupted)
>> -        issued = -1;
>> +    if (!(*issued) && io_interrupted)
>> +        *issued = -1;
>> -    return issued;
>> +    return *issued;
> 
> A little weired that __issue_discard_cmd_orderly() will output the data
> in both return value and @issued parameter.
> 
Got it.
> How about changing type of return value __issue_discard_cmd_orderly()
> from unsigned int to void.
> 
I will sent a v2 patch for this.
> Thanks,
> 
>>   }
>>   static unsigned int __wait_all_discard_cmd(struct f2fs_sb_info *sbi,
>>                       struct discard_policy *dpolicy);
>> @@ -1465,7 +1464,7 @@ static int __issue_discard_cmd(struct 
>> f2fs_sb_info *sbi,
>>               break;
>>           if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered)
>> -            return __issue_discard_cmd_orderly(sbi, dpolicy);
>> +            return __issue_discard_cmd_orderly(sbi, dpolicy, &issued);
>>           pend_list = &dcc->pend_list[i];