Re: [PATCH v3 6/7] sd: Implement support for ZBC devices

["Martin K. Petersen" <martin.petersen@oracle.com>]

>>>>> "Damien" == Damien Le Moal <damien.lemoal@hgst.com> writes:

Damien,

The new stuff looks much cleaner. Thanks for doing that!

This hunk has an unintended side effect:

@@ -2836,14 +2896,14 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	 * react badly if we do.
 	 */
 	if (sdkp->media_present) {
-		sd_read_capacity(sdkp, buffer);
-
 		if (scsi_device_supports_vpd(sdp)) {
 			sd_read_block_provisioning(sdkp);
 			sd_read_block_limits(sdkp);
 			sd_read_block_characteristics(sdkp);
 		}
 
+		sd_read_capacity(sdkp, buffer);
+
 		sd_read_write_protect_flag(sdkp, buffer);
 		sd_read_cache_type(sdkp, buffer);
 		sd_read_app_tag_own(sdkp, buffer);

LMBPE from READ CAPACITY(16) will not be set when calling
sd_read_block_provisioning() and thus we bail early (we catch it second
time around). You may want to split that vpd conditional to shuffle the
block_characteristics on top. Or even better: Split the capacity/block
size printing code from sd_read_capacity() into a separate function and
do:

	if (sdkp->media_present) {
		sd_read_capacity(sdkp, buffer);

		if (sd_try_extended_inquiry(sdp)) {
			sd_read_block_provisioning(sdkp);
			sd_read_block_limits(sdkp);
			sd_read_block_characteristics(sdkp);
		}

+               sd_print_capacity(sdkp);

		sd_read_write_protect_flag(sdkp, buffer);
		sd_read_cache_type(sdkp, buffer);
		sd_read_app_tag_own(sdkp, buffer);
		sd_read_write_same(sdkp, buffer);
	}

Typo:

static int sd_zbc_read_zoned_charateristics(struct scsi_disk *sdkp,
                             ^^^^^^^^^^^^^^

My second comment is that I don't particularly like the notion of values
being stored and passed in units of block layer sectors in struct
scsi_disk and sd*.  As a rule of thumb I prefer SCSI stuff to be counted
in logical blocks and block layer stuff in 512b sectors. You may want to
entertain having a sectors_to_zone() wrapper to facilitate that
conversion.

-- 
Martin K. Petersen	Oracle Linux Engineering