On 7/13/22 02:14, Bart Van Assche wrote:
> On 7/11/22 16:28, Damien Le Moal wrote:
>> On 7/12/22 08:11, Damien Le Moal wrote:
>>> On 7/12/22 08:00, Bart Van Assche wrote:
>>>> Using the RETURNED LOGICAL BLOCK ADDRESS field + 1 as the capacity (largest
>>>> addressable LBA) if RC BASIS = 0 is wrong if there are sequential write
>>>> required zones. Hence only use the RC BASIS = 0 capacity if there are no
>>>> sequential write required zones.
>>>
>>> This does not make sense to me: RC BASIS == 0 is defined like this:
>>>
>>> The RETURNED LOGICAL BLOCK ADDRESS field indicates the highest LBA of a
>>> contiguous range of zones that are not sequential write required zones
>>> starting with the first zone.
>>>
>>> Do you have these cases:
>>> 1) host-managed disks:
>>> SWR zones are *mandatory* so there is at least one. Thus read capacity
>>> will return either 0 if there are no conventional zones (they are
>>> optional) or the total capacity of the set of contiguous conventional
>>> zones starting at lba 0. In either case, read capacity does not give you
>>> the actual total capacity and you have to look at the report zones reply
>>> max lba field.
>>> 2) host-aware disks:
>>> There are no SWR zones, there cannot be any. You can only have
>>> conventional zones (optionally) and sequential write preferred zones. So
>>> "the highest LBA of a contiguous range of zones that are not sequential
>>> write required zones starting with the first zone" necessarily is always
>>> the total capacity. RC BASIS = 0 is non-sensical for host-aware drives.
>>
>> What I meant to say here for the host-aware case is that RC BASIS is
>> irrelevant. Both RC BASIS = 0 and = 1 end up with read capacity giving the
>> actual total capacity. ANd for good reasons: the host-aware model is
>> designed to be backward compatible with regular disks, which do not have
>> RC BASIS, so RC BASIS = 0, always.
> Hi Damien,
>
> I agree that for host-aware block devices (conventional + sequential write
> preferred zones) RC BASIS is irrelevant.
>
> What I'm concerned about is host-managed block devices (conventional + sequential
> write required zones) that report an incorrect RC BASIS value (0 instead of 1).
> Shouldn't sd_zbc_check_capacity() skip the capacity reported by host-managed block
> devices that report RC BASIS = 0?
If that is so much a concern, we could do something like this:
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 6acc4f406eb8..32da54e7b68a 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -716,17 +716,15 @@ static int sd_zbc_check_capacity(struct scsi_disk
*sdkp, unsigned char *buf,
if (ret)
return ret;
- if (sdkp->rc_basis == 0) {
- /* The max_lba field is the capacity of this device */
- max_lba = get_unaligned_be64(&buf[8]);
- if (sdkp->capacity != max_lba + 1) {
- if (sdkp->first_scan)
- sd_printk(KERN_WARNING, sdkp,
- "Changing capacity from %llu to
max LBA+1 %llu\n",
- (unsigned long long)sdkp->capacity,
- (unsigned long long)max_lba + 1);
- sdkp->capacity = max_lba + 1;
- }
+ /* The max_lba field is the capacity of this device */
+ max_lba = get_unaligned_be64(&buf[8]);
+ if (sdkp->capacity != max_lba + 1) {
+ if (sdkp->first_scan)
+ sd_printk(KERN_WARNING, sdkp,
+ "Changing capacity from %llu to max LBA+1
%llu\n",
+ (unsigned long long)sdkp->capacity,
+ (unsigned long long)max_lba + 1);
+ sdkp->capacity = max_lba + 1;
}
if (sdkp->zone_starting_lba_gran == 0) {
That is, always check the reported capacity against max_lba of report
zones reply, regardless of rc_basis (and we can even then drop the
rc_basis field from struct scsi_disk).
But I would argue that any problem with the current code would mean a
buggy device firmware. For such case, the device FW should be fixed or we
should add a quirk for it.
>
> Thanks,
>
> Bart.
--
Damien Le Moal
Western Digital Research
