Re: [PATCH 0/3] Improve libata support for FUA

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 10/22/22 22:50, Hannes Reinecke wrote:
> On 10/22/22 00:45, Damien Le Moal wrote:
>> On 10/22/22 06:02, Maciej S. Szmigiero wrote:
>>> On 21.10.2022 07:38, Damien Le Moal wrote:
>>>> These patches cleanup and improve libata support for the FUA device
>>>> feature. Patch 3 enables FUA support by default for any drive that
>>>> reports supporting the feature.
>>>>
>>>> Damien Le Moal (2):
>>>>     ata: libata: cleanup fua handling
>>>>     ata: libata: Enable fua support by default
>>>>
>>>> Maciej S. Szmigiero (1):
>>>>     ata: libata: allow toggling fua parameter at runtime
>>>>
>>>
>>> Thanks Damien for the series!
>>>
>>> I've looked at the code changes and have basically two points:
>>> 1) There seems to be no way to revalidate the FUA setting for an existing
>>> disk, since it is now only taken into account in ata_dev_config_fua().
>>>
>>> As far as I can see, this function is only called on probe paths
>>> (and during exception handling), so if the "libata.fua" parameter is
>>> toggled the new setting would only affect newly (re-)attached disks.
>>
>> Yes. Indeed. Forcing an ATA revalidation needs some more trickery as the
>> regular sd_revalidate() does not lead to ata_dev_configure() being called
>> again.
>>
> But shouldn't we rather fix that?
> After Johns series of pre-allocating the SCSI devices we should be able 
> to call ata_dev_configure() from sd_revalidate() ...

Yes, that should work. Though I am not sure if we really want to call
ata_dev_configure() every time sd_revalidate() is called, given the
performance impact of going to EH to revalidate an ATA drive. On an
average distro, there are quite a lot of revalidate going on...

For this particular case though, changing libata fua module parameter
value at libata run-time should trigger a revalidate of *all* ata drives,
which is different from the regular per-device revalidate driven by events
or the user changing a drive config through sysfs.


-- 
Damien Le Moal
Western Digital Research