Re: [PATCHv2] nvme: only use power of two io boundaries

[Christoph Hellwig <hch@lst.de>]

On Thu, Aug 27, 2020 at 08:28:24AM -0700, Keith Busch wrote:
> +	if (is_power_of_2(iob) && !blk_queue_is_zoned(ns->queue))
> +		blk_queue_chunk_sectors(ns->queue, iob);
> +	else if (iob && !(disk->flags & GENHD_FL_UP))
> +		dev_warn(ctrl->device, "namespace:%u has unused io boundary:%u\n",
> +			ns->head->ns_id, iob);
> +

Similar to the zoned stuff I just find this super hard to read.

I wonder if we need to untangle this a little.  Something like

static inline bool nvme_first_scan(struct nvme_ns *)
{
	/* add a comment here */
	return !(ns->disk->flags & GENHD_FL_UP)
}

static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id)
{
	u32 chunk_sectors = nvme_lba_to_sect(ns, le16_to_cpu(id->noiob));

	if ((ctrl->quirks & NVME_QUIRK_STRIPE_SIZE) &&
	    is_power_of_2(ctrl->max_hw_sectors))
	    	chunk_sectors = ctrl->max_hw_sectors;

	if (!chunk_sectors)
		return;

	if (!is_power_of_2(chunk_sectors)) {
		if (nvme_first_scan(ns)) {
			dev_warn(ctrl->device,
				"namespace%u: ignoring unaligned I/O boundary: %u\n",
				ns->head->ns_id, chunk_sectors);
		}
		return;
	}

	if (blk_queue_is_zoned(ns->queue)) {
		if (nvme_first_scan(ns)) {
			dev_info(ctrl->device,
				"namespace%u: ignoring I/O boundary for zoned namespace\n",
				ns->head->ns_id);
		}
		return;
	}
			
	blk_queue_chunk_sectors(ns->queue, chunk_sectors);
}

>  	nvme_update_disk_info(disk, ns, id);
>  #ifdef CONFIG_NVME_MULTIPATH
>  	if (ns->head->disk) {
> -- 
> 2.24.1
---end quoted text---

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