[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