Bart,
On 4/5/18 00:08, Bart Van Assche wrote:
> On 04/04/18 01:54, Damien Le Moal wrote:
>> static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
>> {
>> + u64 sdkp_zone_blocks = sdkp->zone_blocks;
>
> Shouldn't this variable be initialized to zero such that zone size
> changes are accepted even if the SAME field in the REPORT ZONES response
> is zero?
sdkp_zone_blocks will be 0 when sd_zbc_check_zone_size() is called for
the first scan of a disk and will hold the current disk value if
sd_zbc_check_zone_size() is called on a revalidate after first scan. If
the initial value is 0, there is no check and the variable is first
initialized. Otherwise, the value is compared to the zone size reported.
In both cases, the zone size change will be cought.
But granted, setting the value initially to 0 is easier to understand.
I will also change:
} else {
sdkp->zone_blocks = zone_blocks;
sdkp->zone_shift = ilog2(zone_blocks);
}
to
} else if (sdkp->zone_blocks != zone_blocks) {
sdkp->zone_blocks = zone_blocks;
sdkp->zone_shift = ilog2(zone_blocks);
}
to make things really clear.
Similarly to a capacity change, It may also be good to add a warning
message. After all, on a drive swap, we can have the case where the
capacity does not change, but the zone size does.
Sending a v2.
Best regards.
--
Damien Le Moal,
Western Digital