Re: [PATCHv3] nvme: skip noiob for zoned devices

[Christoph Hellwig <hch@lst.de>]

On Tue, Aug 18, 2020 at 10:50:54AM -0700, Keith Busch wrote:
> 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?

True, might make sense to add a validity check instead.

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

Ok.  Maybe we just need to move the whole thing into a helper to look
less confusing.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme