On 10/21/22 10:00, Damien Le Moal wrote:
On 10/21/22 15:50, Damien Le Moal wrote:
On 10/21/22 15:21, Hannes Reinecke wrote:
On 10/21/22 07:38, Damien Le Moal wrote:
From: "Maciej S. Szmigiero" <maciej.szmigiero@xxxxxxxxxx>
Currently, the libata.fua parameter isn't runtime-writable, so a
system restart is required in order to toggle it.
This unnecessarily complicates testing how drives behave with FUA on and
off.
Let's make this parameter R/W instead, like many others in the kernel.
Example usage:
Disable the parameter:
echo 0 >/sys/module/libata/parameters/fua
Revalidate disk cache settings:
F=/sys/class/scsi_disk/0\:0\:0\:0/cache_type; echo `cat $F` >$F
[Damien]
Enabling fua support by setting libata.fua to 1 will have no effect if
the libata module is loaded with libata.force=[ID]nofua, which disables
fua support for the ata device(s) identified with ID or all ata devices
if no ID is specified.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@xxxxxxxxxx>
Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
---
drivers/ata/libata-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6008f7ed1c42..1bb9616b10d9 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -128,7 +128,7 @@ module_param(atapi_passthru16, int, 0444);
MODULE_PARM_DESC(atapi_passthru16, "Enable ATA_16 passthru for ATAPI devices (0=off, 1=on [default])");
int libata_fua = 0;
-module_param_named(fua, libata_fua, int, 0444);
+module_param_named(fua, libata_fua, int, 0644);
MODULE_PARM_DESC(fua, "FUA support (0=off [default], 1=on)");
static int ata_ignore_hpa;
Hmm. I guess you'll need to revalidate the drive when changing that; but
this can be done in a later patch.
Well, this is not sysfs, we cannot do this automatically easily...
And thinking about it now that you mention it, going from fua=1 to fua=0
can actually cause problems. The reverse not, since scsi side would still
see fua=0 until revalidation.
So... Unless we find a way to link the param write to reavlidation, we
should actually not allow this.
Maciej ? Thoughts ?
I looked at this a little more. We could define the operations (struct
kernel_param_ops) manually together with the fua parameter declaration,
but that would be really ugly...
Given that we are switching to fua=1 by default, do you still need a
dynamic argument ? I am now thinking that this patch should be dropped.
I'd kill it, and let users it handle via blacklist flags only.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
