On 07/22/2016 11:56 PM, Ewan D. Milne wrote: > 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@xxxxxxxx> > > ... > >> +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? > The ZBC spec mandates that the zone size must be a power of 2. So I don't have problems with triggering a BUG_ON for non-compliant drives. Cheers, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html