RE: [PATCH v3] f2fs: deliver the accumulated 'issued' to __issue_discard_cmd_orderly() to meet the max_requests limit

[管玉伟(竹官)]

> 
> 
> On 2022/12/13 17:34, Yuwei Guan wrote:
> > Any of the following scenarios will send more than the number of
> > max_requests at a time, which will not meet the design of the
> > max_requests limit.
> >
> > - Set max_ordered_discard larger than discard_granularity from userspace.
> > - It is a small size device, discard_granularity can be tuned to 1 in
> >    f2fs_tuning_parameters().
> >
> > We need to deliver the accumulated @issued to
> > __issue_discard_cmd_orderly() to meet the max_requests limit.
> >
> > BTW, convert the parameter type of @issued in __submit_discard_cmd().
> >
> > Signed-off-by: Yuwei Guan <Yuwei.Guan@xxxxxxxxxxxxx>
> > Cc: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
> 
> For the code part, it looks good to me.
> 
> Reviewed-by: Chao Yu <chao@xxxxxxxxxx>
> 
Hi Jeageuk,

Could you help to review this commit.

Thanks.
> Thanks,
> 
> > ---
> >   fs/f2fs/segment.c | 24 +++++++++++-------------
> >   1 file changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index
> > a9099a754dd2..5268938466f5 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1097,8 +1097,7 @@ static void __update_discard_tree_range(struct
> f2fs_sb_info *sbi,
> >   /* this function is copied from blkdev_issue_discard from block/blk-lib.c
> */
> >   static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
> >                                               struct discard_policy *dpolicy,
> > -                                             struct discard_cmd *dc,
> > -                                             unsigned int *issued)
> > +                                             struct discard_cmd *dc,
> > + int *issued)
> >   {
> >       struct block_device *bdev = dc->bdev;
> >       unsigned int max_discard_blocks = @@ -1379,8 +1378,8 @@ static
> > void __queue_discard_cmd(struct f2fs_sb_info *sbi,
> >       mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
> >   }
> >
> > -static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
> > -                                     struct discard_policy *dpolicy)
> > +static void __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
> > +1387,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 +1413,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 +1431,8 @@
> > static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info
> > *sbi,
> >
> >       mutex_unlock(&dcc->cmd_lock);
> >
> > -     if (!issued && io_interrupted)
> > -             issued = -1;
> > -
> > -     return issued;
> > +     if (!(*issued) && io_interrupted)
> > +             *issued = -1;
> >   }
> >   static unsigned int __wait_all_discard_cmd(struct f2fs_sb_info *sbi,
> >                                       struct discard_policy *dpolicy);
> > @@ -1464,8 +1460,10 @@ static int __issue_discard_cmd(struct
> f2fs_sb_info *sbi,
> >               if (i + 1 < dpolicy->granularity)
> >                       break;
> >
> > -             if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered)
> > -                     return __issue_discard_cmd_orderly(sbi, dpolicy);
> > +             if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered) {
> > +                     __issue_discard_cmd_orderly(sbi, dpolicy, &issued);
> > +                     return issued;
> > +             }
> >
> >               pend_list = &dcc->pend_list[i];
> >