On Aug 27, 2019 / 09:34, Chao Yu wrote:
> On 2019/8/21 12:47, Shin'ichiro Kawasaki wrote:
> > To prepare for write pointer consistency check by fsck, add
> > f2fs_report_zones() helper function which calls REPORT ZONE command to
> > get write pointer status. The function is added to lib/libf2fs_zoned
> > which gathers zoned block device related functions.
> > 
> > To check write pointer consistency with f2fs meta data, fsck needs to
> > refer both of reported zone information and f2fs super block structure
> > "f2fs_sb_info". However, libf2fs_zoned does not import f2fs_sb_info. To
> > keep f2fs_sb_info structure out of libf2fs_zoned, provide a callback
> > function in fsck to f2fs_report_zones() and call it for each zone.
> > 
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com>
> > ---
> >  include/f2fs_fs.h   |  2 ++
> >  lib/libf2fs_zoned.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+)
> > 
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index 0d9a036..abadd1b 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -1279,6 +1279,8 @@ blk_zone_cond_str(struct blk_zone *blkz)
> >  
> >  extern int f2fs_get_zoned_model(int);
> >  extern int f2fs_get_zone_blocks(int);
> > +typedef int (report_zones_cb_t)(int i, struct blk_zone *blkz, void 
> > *opaque);
> > +extern int f2fs_report_zones(int, report_zones_cb_t *, void *);
> >  extern int f2fs_check_zones(int);
> >  extern int f2fs_reset_zones(int);
> >  
> > diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> > index af00b44..fc4974f 100644
> > --- a/lib/libf2fs_zoned.c
> > +++ b/lib/libf2fs_zoned.c
> > @@ -193,6 +193,57 @@ int f2fs_get_zone_blocks(int i)
> >  
> >  #define F2FS_REPORT_ZONES_BUFSZ    524288
> >  
> > +int f2fs_report_zones(int j, report_zones_cb_t *report_zones_cb, void 
> > *opaque)
> > +{
> > +   struct device_info *dev = c.devices + j;
> > +   struct blk_zone_report *rep;
> > +   struct blk_zone *blkz;
> > +   unsigned int i, n = 0;
> > +   u_int64_t total_sectors = (dev->total_sectors * c.sector_size) >> 9;
> 
> Hi Shin'ichiro,

Hi Chao, thank you for your review.

> Could we use SECTOR_SHIFT instead?

Yes. In the third patch, I added SECTOR_SHIFT definition in fsck/fsck.c. To use
it both in lib/libf2fs_zoned.c and fsck/fsck.c, I will define SECTOR_SHIFT in
include/f2fs_fs.h.

> 
> > +   u_int64_t sector = 0;
> > +   int ret = -1;
> > +
> > +   rep = malloc(F2FS_REPORT_ZONES_BUFSZ);
> > +   if (!rep) {
> > +           ERR_MSG("No memory for report zones\n");
> > +           return -ENOMEM;
> > +   }
> > +
> > +   while (sector < total_sectors) {
> > +
> > +           /* Get zone info */
> > +           rep->sector = sector;
> > +           rep->nr_zones = (F2FS_REPORT_ZONES_BUFSZ - sizeof(struct 
> > blk_zone_report))
> > +                   / sizeof(struct blk_zone);
> > +
> > +           ret = ioctl(dev->fd, BLKREPORTZONE, rep);
> > +           if (ret != 0) {
> > +                   ret = -errno;
> > +                   ERR_MSG("ioctl BLKREPORTZONE failed\n");
> It's minor, it will be better to print errno here, since I didn't see we print
> error no from caller.

Thanks. Will do that.

> 
> > +                   goto out;
> > +           }
> > +
> > +           if (!rep->nr_zones) {
> > +                   ret = -EIO;
> > +                   ERR_MSG("Unexpected ioctl BLKREPORTZONE result\n");
> > +                   goto out;
> > +           }
> > +
> > +           blkz = (struct blk_zone *)(rep + 1);
> > +           for (i = 0; i < rep->nr_zones && sector < total_sectors; i++) {
> 
> The condition looks like that block zones in rep may across end-of-device? 
> Will
> this really happen?
> 
> So I mean will "i < rep->nr_zones" be enough?

You are correct. I will remove the sector comparison in v2 series.

--
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