[PATCHv3] nvme: skip noiob for zoned devices
Keith Busch
kbusch at kernel.org
Tue Aug 18 13:50:54 EDT 2020
On Tue, Aug 18, 2020 at 07:25:04PM +0200, Christoph Hellwig wrote:
> > - if (iob)
> > - blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > + if (iob) {
> > + if (!blk_queue_is_zoned(disk->queue))
> > + blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > + else if (!(disk->flags & GENHD_FL_UP))
> > + pr_warn("%s: zone namespace has unused IO boundary:%u\n",
> > + disk->disk_name, iob);
>
> This has some overly long line and looks fairly convoluted, what
> about something like:
>
> if (iob) {
> unsigned int chunk_sectors = rounddown_pow_of_two(iob);
Unrelated to this particular patch, but ... why is this rounddown here?
While we can't use chunk_sectors if it is not a power of two, but what
benefit are we expecting to get from using the wrong boundary just
because it's convenient to our block stack?
> if (blk_queue_is_zoned(disk->queue) &&
> chunk_sectors > blk_queue_zone_sectors(disk->queue))
I don't think we should skip iob just for the case where chunk_sectors
is the larger value. A smaller value still ruins things like
blk_queue_zone_sectors() and blk_queue_zone_no().
Also, we call blk_revalidate_disk_zones() after the iob check, so the
blk_queue_zone_sectors() hasn't been initialized for this revalidate.
> pr_warn("%s: ignoring too large IO boundary: %u\n",
> disk->disk_name, iob);
We don't want to emit a repeated warning on every disk revalidation, so
I added the check for GENHD_FL_UP to warn only on the very first disk
validation.
> else
> blk_queue_chunk_sectors(ns->queue, chunk_sectors);
> }
More information about the Linux-nvme
mailing list