Re: [PATCH 1/5] sd: configure ZBC devices

["Ewan D. Milne" <emilne@redhat.com>]

On Tue, 2016-07-19 at 15:25 +0200, Hannes Reinecke wrote:
> For ZBC devices I/O must not cross zone boundaries, so setup
> the 'chunk_sectors' block queue setting to the zone size.
> This is only valid for REPORT ZONES SAME type 2 or 3;
> for other types the zone sizes might be different
> for individual zones. So issue a warning if the type is
> found to be different.
> Also the capacity might be different from the announced
> capacity, so adjust it as needed.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>

...

> +static void sd_read_zones(struct scsi_disk *sdkp, unsigned char *buffer)
> +{
> +	int retval;
> +	unsigned char *desc;
> +	u32 rep_len;
> +	u8 same;
> +	u64 zone_len, lba;
> +
> +	if (sdkp->zoned != 1)
> +		/* Device managed, no special handling required */
> +		return;
> +
> +	retval = sd_zbc_report_zones(sdkp, buffer, SD_BUF_SIZE,
> +				     0, ZBC_ZONE_REPORTING_OPTION_ALL, false);
> +	if (retval < 0)
> +		return;
> +
> +	rep_len = get_unaligned_be32(&buffer[0]);
> +	if (rep_len < 64) {
> +		sd_printk(KERN_WARNING, sdkp,
> +			  "REPORT ZONES report invalid length %u\n",
> +			  rep_len);
> +		return;
> +	}
> +
> +	if (sdkp->rc_basis == 0) {
> +		/* The max_lba field is the capacity of a zoned device */
> +		lba = get_unaligned_be64(&buffer[8]);
> +		if (lba + 1 > sdkp->capacity) {
> +			sd_printk(KERN_WARNING, sdkp,
> +				  "Max LBA %zu (capacity %zu)\n",
> +				  (sector_t) lba + 1, sdkp->capacity);
> +			sdkp->capacity = lba + 1;
> +		}
> +	}
> +
> +	/*
> +	 * Adjust 'chunk_sectors' to the zone length if the device
> +	 * supports equal zone sizes.
> +	 */
> +	same = buffer[4] & 0xf;
> +	if (same == 0 || same > 3) {
> +		sd_printk(KERN_WARNING, sdkp,
> +			  "REPORT ZONES SAME type %d not supported\n", same);
> +		return;
> +	}
> +	/* Read the zone length from the first zone descriptor */
> +	desc = &buffer[64];
> +	zone_len = logical_to_sectors(sdkp->device,
> +				      get_unaligned_be64(&desc[8]));
> +	blk_queue_chunk_sectors(sdkp->disk->queue, zone_len);
> +}
> +

So, blk_queue_chunk_sectors() has:

void blk_queue_chunk_sectors(struct request_queue *q, unsigned int chunk_sectors)
{
        BUG_ON(!is_power_of_2(chunk_sectors));
        q->limits.chunk_sectors = chunk_sectors;
}

and it seems like if some device reports a non-power-of-2 zone_len then we
will BUG_ON().  Probably would be better if we reported an error instead?

-Ewan