From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] ata: allow enabling FUA support in Kconfig
Date: Thu, 6 Oct 2022 08:38:38 +0900 [thread overview]
Message-ID: <24a48f71-8a79-6311-1e43-494df0458a32@opensource.wdc.com> (raw)
In-Reply-To: <df8701f3905c1a394863e57c7a2d30c5b5dc3503.1664221825.git.maciej.szmigiero@oracle.com>
On 9/27/22 04:51, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>
> Currently, if one wants to make use of FUA support in libata it is
> necessary to provide an explicit kernel command line parameter in order to
> enable it (for drives that report such support).
>
> In terms of Git archaeology: FUA support was enabled by default in early
> libata versions but was disabled soon after.
> Since then there were a few attempts to enable this support by default:
> [1] (for NCQ drives only), [2] (for all drives).
> However, the second change had to be reverted after a report came of
> an incompatibility with the HDD in 2011 Mac Mini.
>
> Enabling FUA avoids having to emulate it by issuing an extra drive cache
> flush for every request that have this flag set.
> Since FUA support is required by the ATA/ATAPI spec for any drive that
> supports LBA48 and so these days should be pretty widespread let's provide
> an ability to enable it by default in Kconfig.
This can be done by adding "libata.fua=1" to the CONFIG_CMDLINE option. So
I do not see the need to add yet another config option.
Patch 1 looks good. I will queue it up once rc1 is out.
>
> [1]: https://lore.kernel.org/lkml/45CFFF82.4030301@shaw.ca/
> [2]: https://lore.kernel.org/lkml/1336447443-4685-1-git-send-email-wenqing.lz@taobao.com/
>
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
> drivers/ata/Kconfig | 15 +++++++++++++++
> drivers/ata/libata-core.c | 5 +++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 36833a8629980..fd39bb22963a3 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -102,6 +102,21 @@ config SATA_PMP
> This option adds support for SATA Port Multipliers
> (the SATA version of an ethernet hub, or SAS expander).
>
> +config ATA_FUA
> + bool "ATA FUA support"
> + help
> + Enables Forced Unit Access (FUA) support by default for drives that
> + have it.
> + Otherwise the FUA flag has to be emulated by flushing the drive cache.
> +
> + Regardless of this option, you can enable or disable such support at
> + kernel boot time by providing libata.fua=1 or libata.fua=0 kernel
> + command line parameter.
> +
> + If building a kernel for yourself, say Y.
> + If building a kernel for a distro that's supposed to run out of the
> + box on old and broken hardware say N.
> +
> if HAS_DMA
>
> comment "Controllers with non-SFF native interface"
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index b322006c85806..941836c4b5eda 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -127,9 +127,10 @@ int atapi_passthru16 = 1;
> 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;
> +int libata_fua = IS_ENABLED(CONFIG_ATA_FUA);
> module_param_named(fua, libata_fua, int, 0644);
> -MODULE_PARM_DESC(fua, "FUA support (0=off [default], 1=on)");
> +MODULE_PARM_DESC(fua,
> + "FUA support (0=off, 1=on), default " __stringify(IS_ENABLED(CONFIG_ATA_FUA)));
>
> static int ata_ignore_hpa;
> module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644);
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2022-10-05 23:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 19:51 [PATCH 1/2] ata: allow toggling fua parameter at runtime Maciej S. Szmigiero
2022-09-26 19:51 ` [PATCH 2/2] ata: allow enabling FUA support in Kconfig Maciej S. Szmigiero
2022-10-05 23:38 ` Damien Le Moal [this message]
2022-10-06 13:06 ` Maciej S. Szmigiero
2022-10-06 22:20 ` Damien Le Moal
2022-10-06 22:53 ` Damien Le Moal
2022-10-07 14:14 ` Maciej S. Szmigiero
2022-10-07 22:41 ` Damien Le Moal
2022-10-14 16:44 ` Maciej S. Szmigiero
2022-10-15 0:37 ` Damien Le Moal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=24a48f71-8a79-6311-1e43-494df0458a32@opensource.wdc.com \
--to=damien.lemoal@opensource.wdc.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mail@maciej.szmigiero.name \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).