> Il giorno 26 lug 2021, alle ore 13:33, Damien Le Moal <Damien.LeMoal@xxxxxxx> ha scritto: > > 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. > 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. Thanks, Paolo >> >>>> 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