Re: [PATCH v3 1/4] block: Add concurrent positioning ranges support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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?

Hmm. Not sure if I like that, but then it might be the best option after all. So you can add my:

Reviewed-by: Hannes Reinecke <hare@xxxxxxx>

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux