Re: [dm-devel] [PATCH 1/4] block: Enhance blk_revalidate_disk_zones()
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 4bc5f260248a..293891b7068a 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -441,6 +441,57 @@ void blk_queue_free_zone_bitmaps(struct request_queue *q) > q->seq_zones_wlock = NULL; > } > > +/** > + * blk_check_zone - Check a zone information > + * @q: request queue > + * @zone:the zone to check > + * @sector: start sector of the zone > + * > + * Helper function to check zones of a zoned block device. Returns true if > the > + * zone is correct and false if a problem is detected. > + */ > +static bool blk_check_zone(struct gendisk *disk, struct blk_zone *zone, > +sector_t *sector) Maybe call this blk_zone_valid? Also in many places we don't really do the verbose kerneldoc comments with the duplicated parameter names for local functions, as the scripts only pick up non-stack ones anyway. > + /* > + * All zones must have the same size, with the exception on an eventual > + * smaller last zone. > + */ > + if (zone->start + zone_sectors < capacity && > + zone->len != zone_sectors) { > + pr_warn("%s: Invalid zone device with non constant zone size\n", > + disk->disk_name); > + return false; > + } I think we should also move the power of two zone size check here instead of leaving it in the driver. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/4] block: Enhance blk_revalidate_disk_zones()
On Thu, Oct 24, 2019 at 03:50:03PM +0900, Damien Le Moal wrote: > - /* Do a report zone to get max_lba and the same field */ > - ret = sd_zbc_do_report_zones(sdkp, buf, bufsize, 0, false); > + /* Do a report zone to get max_lba and the size of the first zone */ > + ret = sd_zbc_do_report_zones(sdkp, buf, SD_BUF_SIZE, 0, false); This is no longer reading all the zones here, so you could set the 'partial' field to true. And then since this was the only caller that had set partial to false, you can also remove that parameter. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/4] block: Enhance blk_revalidate_disk_zones()
On 2019/10/24 16:08, Christoph Hellwig wrote: >> diff --git a/block/blk-zoned.c b/block/blk-zoned.c >> index 4bc5f260248a..293891b7068a 100644 >> --- a/block/blk-zoned.c >> +++ b/block/blk-zoned.c >> @@ -441,6 +441,57 @@ void blk_queue_free_zone_bitmaps(struct request_queue >> *q) >> q->seq_zones_wlock = NULL; >> } >> >> +/** >> + * blk_check_zone - Check a zone information >> + * @q: request queue >> + * @zone: the zone to check >> + * @sector: start sector of the zone >> + * >> + * Helper function to check zones of a zoned block device. Returns true if >> the >> + * zone is correct and false if a problem is detected. >> + */ >> +static bool blk_check_zone(struct gendisk *disk, struct blk_zone *zone, >> + sector_t *sector) > > Maybe call this blk_zone_valid? Also in many places we don't really > do the verbose kerneldoc comments with the duplicated parameter names > for local functions, as the scripts only pick up non-stack ones anyway. > >> +/* >> + * All zones must have the same size, with the exception on an eventual >> + * smaller last zone. >> + */ >> +if (zone->start + zone_sectors < capacity && >> +zone->len != zone_sectors) { >> +pr_warn("%s: Invalid zone device with non constant zone size\n", >> +disk->disk_name); >> +return false; >> +} > > I think we should also move the power of two zone size check here > instead of leaving it in the driver. We should not since the drivers calls blk_queue_chunk_sectors() first to set the zone size and that function has a power of 2 check. So better check that earlier than here. -- Damien Le Moal Western Digital Research -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel