On 2021/07/26 17:47, Hannes Reinecke wrote:
> On 7/26/21 10:30 AM, Damien Le Moal wrote:
>> On 2021/07/26 16:34, Hannes Reinecke wrote:
> [ .. ]
>>> In principle it looks good, but what would be the appropriate action
>>> when invalid ranges are being detected during revalidation?
>>> The current code will leave the original ones intact, but I guess that's
>>> questionable as the current settings are most likely invalid.
>>
>> Nope. In that case, the old ranges are removed. In blk_queue_set_cranges(),
>> there is:
>>
>> + if (!blk_check_ranges(disk, cr)) {
>> + kfree(cr);
>> + cr = NULL;
>> + goto reg;
>> + }
>>
>> So for incorrect ranges, we will register "NULL", so no ranges. The old ranges
>> are gone.
>>
>
> Right. So that's the first concern addressed.
Not that at the scsi layer, if there is an error retrieving the ranges
informations, blk_queue_set_cranges(q, NULL) is called, so the same happen: the
ranges set are removed and no range information will appear in sysfs.
>
>>> I would vote for de-register the old ones and implement an error state
>>> (using an error pointer?); that would signal that there _are_ ranges,
>>> but we couldn't parse them properly.
>>> Hmm?
>>
>> With the current code, the information "there are ranges" will be completely
>> gone if the ranges are bad... dmesg will have a message about it, but that's it.
>>
> So there will be no additional information in sysfs in case of incorrect
> ranges?
Yep, there will be no queue/cranges directory. The drive will be the same as a
single actuator one.
> Hmm. Not sure if I like that, but then it might be the best option after
> all. So you can add my:
Nothing much that we can do. If we fail to retrieve the ranges, or the ranges
are incorrect, access optimization by FS or scheduler is not really possible.
Note that the drive will still work. Only any eventual optimization will be
turned off.
> Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
Thanks !
>
> Cheers,
>
> Hannes
>
--
Damien Le Moal
Western Digital Research
