Re: [PATCH 09/14] scsi: sd: Fix discard errors during revalidate

Bart Van Assche
2022-03-06T00:35:23Z

On 3/3/22 19:55, Martin K. Petersen wrote:
>
> Bart,
>
>>> + if (mode == SD_LBP_DEFAULT && !sdkp->provisioning_override) {
>>
>> Hmm ... is provisioning_override ever true for the SD_LBP_DEFAULT
>> mode? If not, can "&& !sdkp->provisioning_override" be left out?
>
> The two *_override variables are used to prevent subsequent revalidates
> from clobbering the mode configured by the user.
>
> I experimented with various approaches for this, including having a
> separate SD_LBP_ mode for revalidate, using first_scan, etc. In the end
> I felt that the boolean was the best approach to capturing the fact that
> the currently active mode was explicitly configured by the user.
>
> Open to suggestions.

Hi Martin,

It took me a while before I realized that the purpose of the
provisioning_override variable is to make the behavior of the
sd_config_discard(sdkp, SD_LBP_DEFAULT) different depending on whether the
SD_LBP_DEFAULT comes from userspace (via sysfs) or from the sd_config_discard()
call in sd_revalidate_disk().

How about storing the mode selected by the user inside the
provisioning_mode_store() function and using that variable inside
sd_config_discard() instead of sdkp->provisioning_override? I think that would
result in easier to comprehend code.

Thanks,

Bart.