On Oct 22, 2019 / 17:24, Chao Yu wrote:
> On 2019/10/22 17:10, Damien Le Moal wrote:
> > On 2019/10/22 17:59, Chao Yu wrote:
> >> On 2019/10/18 14:37, Shin'ichiro Kawasaki wrote:
> >>> To prepare for write pointer consistency fix by fsck, add
> >>> f2fs_reset_zone() helper function which calls RESET ZONE command. The
> >>> function is added to lib/libf2fs_zoned which gathers zoned block device
> >>> related functions.
> >>>
> >>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com>
> >>> ---
> >>>  include/f2fs_fs.h   |  1 +
> >>>  lib/libf2fs_zoned.c | 26 ++++++++++++++++++++++++++
> >>>  2 files changed, 27 insertions(+)
> >>>
> >>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> >>> index 1f7ef05..a36927b 100644
> >>> --- a/include/f2fs_fs.h
> >>> +++ b/include/f2fs_fs.h
> >>> @@ -1303,6 +1303,7 @@ extern int f2fs_report_zone(int, u_int64_t, void *);
> >>>  typedef int (report_zones_cb_t)(int i, void *, void *);
> >>>  extern int f2fs_report_zones(int, report_zones_cb_t *, void *);
> >>>  extern int f2fs_check_zones(int);
> >>> +int f2fs_reset_zone(int, void *);
> >>>  extern int f2fs_reset_zones(int);
> >>>  
> >>>  #define SIZE_ALIGN(val, size)    ((val) + (size) - 1) / (size)
> >>> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> >>> index 10d6d0b..1335038 100644
> >>> --- a/lib/libf2fs_zoned.c
> >>> +++ b/lib/libf2fs_zoned.c
> >>> @@ -388,6 +388,26 @@ out:
> >>>   return ret;
> >>>  }
> >>>  
> >>> +int f2fs_reset_zone(int i, void *blkzone)
> >>> +{
> >>> + struct blk_zone *blkz = (struct blk_zone *)blkzone;
> >>> + struct device_info *dev = c.devices + i;
> >>> + struct blk_zone_range range;
> >>> + int ret;
> >>> +
> >>> + if (!blk_zone_seq(blkz) || blk_zone_empty(blkz))
> >>> +         return 0;
> >>> +
> >>> + /* Non empty sequential zone: reset */
> >>> + range.sector = blk_zone_sector(blkz);
> >>> + range.nr_sectors = blk_zone_length(blkz);
> >>> + ret = ioctl(dev->fd, BLKRESETZONE, &range);
> >>> + if (ret != 0)
> >>
> >> As you did in other zoned block device code, errno would be preferred as 
> >> return
> >> value?

Yes, should return -errno. This made me think that it's the better to print
errno in ERR_MSG below. Will reflect these changes in the next version.

> >>
> >>> +         ERR_MSG("ioctl BLKRESETZONE failed\n");
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>>  int f2fs_reset_zones(int j)
> >>>  {
> >>>   struct device_info *dev = c.devices + j;
> >>> @@ -491,6 +511,12 @@ int f2fs_check_zones(int i)
> >>>   return -1;
> >>>  }
> >>>  
> >>> +int f2fs_reset_zone(int i, void *blkzone)
> >>> +{
> >>> + ERR_MSG("%d: Zoned block devices are not supported\n", i);
> >>
> >> Minor thing:
> >>
> >> "device is"?
> > 
> >     ERR_MSG("%d: Unsupported zoned block device\n", i);
> > 
> > may be better.
> 
> Looks better.

Thanks. Will reflect in the next version. Same change will be applied to
1st and 2nd patches for f2fs_report_zones() and f2fs_report_zone().

> 
> > 
> > Note that we should never hit this anyway since the validity of the set
> > of devices used should have been checked way before we start resetting
> > zones.
> 
> Yup.
> 
> Thanks,
> 
> > 
> >>
> >>> + return -1;
> >>> +}
> >>> +
> >>>  int f2fs_reset_zones(int i)
> >>>  {
> >>>   ERR_MSG("%d: Zoned block devices are not supported\n", i);
> >>
> >> "device is"?
> > 
> > Same as above.

Not only f2fs_reset_zones() but also f2fs_check_zones() have the same
ERR_MSG string. I will replace the message string of these functions
with suggested one in this 3rd patch.

Thanks!

--
Best Regards,
Shin'ichiro Kawasaki

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to