On 07/24/2016 12:04 AM, Bart Van Assche wrote:
> On 07/23/16 13:31, Hannes Reinecke wrote:
>> 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.
>
> Triggering BUG_ON() if zone_len is not a power of two is completely
> unacceptable. No matter what zone information a ZBC drive exports that
> shouldn't result in a kernel oops.
>
Ok, will be fixing this.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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
