On 2021/07/27 23:07, Paolo Valente wrote:

quoted

Il giorno 26 lug 2021, alle ore 13:33, Damien Le Moal [off-list ref] ha scritto:

On 2021/07/26 17:47, Hannes Reinecke wrote:

quoted

On 7/26/21 10:30 AM, Damien Le Moal wrote:

quoted

On 2021/07/26 16:34, Hannes Reinecke wrote:

[ .. ]

quoted

quoted

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.

As a very personal opinion, silent failures are often misleading when
trying to understand what is going wrong in a system.  But I guess
this is however the best option.

Failure are not silent: error messages are printed and will be visible in dmesg.

We can always completely ignore the drive and completely fail its initialization
in the case of a failed cranges initialization. But I find that rather extreme
since the drive is supposed to work anyway, even without any access optimization
for the multiple cranges case.

Thanks,
Paolo

quoted

quoted

quoted

quoted

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.

quoted

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.

quoted

Reviewed-by: Hannes Reinecke <hare@suse.de>

Thanks !

quoted

Cheers,

Hannes

-- 
Damien Le Moal
Western Digital Research

-- 
Damien Le Moal
Western Digital Research