Linux EFI development
 help / color / mirror / Atom feed
* [PATCH v3] efivarfs: add nostatfs mount option to skip QueryVariableInfo()
@ 2026-09-25 11:31 Prashant Singh
  2026-09-25 12:14 ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Prashant Singh @ 2026-09-25 11:31 UTC (permalink / raw)
  To: Ard Biesheuvel, Jeremy Kerr
  Cc: Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Jonathan Corbet, Shuah Khan, Randy Dunlap,
	Luis Claudio R . Goncalves, Steve McIntyre, linux-efi,
	linux-kernel, linux-doc, linux-rt-devel, Prashant Singh

QueryVariableInfo() is an EFI runtime service that, on some firmware,
takes tens of milliseconds and runs with preemption disabled, stalling
the CPU that services it. efivarfs_statfs() calls it (rate-limited since
commit b2326338dc68 ("efivarfs: Rate limit statfs() handler")) to report
the variable-store used/available capacity, so any statfs(2) -- e.g.
every "df" -- can inject that stall into unrelated latency-sensitive
workloads on the same CPU.

Add a negatable "nostatfs" mount option: with nostatfs, statfs(2) skips
QueryVariableInfo() and reports zero used/available; with statfs it
reports the capacity as before. It defaults to nostatfs on
CONFIG_PREEMPT_RT so real-time kernels do not take the stall out of the
box, but statfs can be passed there to force reporting back on -- e.g.
for tools such as fwupd that need the efivars free space to update Secure
Boot key databases.

The option can also be toggled on a live mount via remount, so reporting
can be enabled only for the duration of a firmware update without
unmounting the boot-time efivarfs mount:

  mount -o remount,statfs   /sys/firmware/efi/efivars   # reporting on
  mount -o remount,nostatfs /sys/firmware/efi/efivars   # reporting off

Tested on an Intel Xeon E5-2628L v4, 6.12 kernel, via
"strace -T -e trace=statfs df" on the efivarfs mount.

Cost of the firmware call (CONFIG_PREEMPT_RT not set, so reporting is
enabled by default). Default mount calls QueryVariableInfo() and returns
the real store capacity, ~66-129 ms per call:

  statfs("/sys/firmware/efi/efivars", {f_type=EFIVARFS_MAGIC,
      f_bsize=1, f_blocks=90024, f_bfree=33387, f_bavail=28267, ...})
      = 0 <0.129124>

With -o nostatfs the firmware call is skipped, capacity reported as
zero, ~11 us per call:

  statfs("/sys/firmware/efi/efivars", {f_type=EFIVARFS_MAGIC,
      f_bsize=1, f_blocks=0, f_bfree=0, f_bavail=0, ...}) = 0 <0.000011>

PREEMPT_RT default and live remount toggle (CONFIG_PREEMPT_RT=y). The
boot-time mount defaults to nostatfs (mount shows nostatfs); no firmware
call, capacity zero:

  statfs(... f_blocks=0, f_bfree=0, f_bavail=0, ...) = 0 <0.000018>

Enabling reporting in place with "mount -o remount,statfs" (mount now
shows statfs) takes the ~70 ms firmware call and returns the real
capacity, without unmounting:

  statfs(... f_blocks=90024, f_bfree=30315, f_bavail=25195, ...)
      = 0 <0.070293>

Disabling it again with "mount -o remount,nostatfs" returns to the fast,
zero-capacity path:

  statfs(... f_blocks=0, f_bfree=0, f_bavail=0, ...) = 0 <0.000014>

An unrelated remount that does not respecify the option preserves it: a
"mount -o remount,rw" after "remount,statfs" leaves the mount showing
statfs.

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Prashant Singh <singhpra@juniper.net>
---
Changes since v2:
- Make the flag negatable (fsparam_flag_no): "statfs" forces reporting
  back on, "nostatfs" skips QueryVariableInfo(). Default stays nostatfs
  on PREEMPT_RT. This lets fwupd get the efivars free space it needs for
  Secure Boot key (KEK/DB/DBX) updates on an RT kernel by mounting with
  -o statfs (Luis Claudio R. Goncalves, Steve McIntyre).
- Support toggling the option on a live mount via remount, so reporting
  can be enabled just for a firmware update without unmounting efivarfs
  (Sebastian Andrzej Siewior). Options not respecified on remount are
  preserved.
- Read/write nostatfs with READ_ONCE()/WRITE_ONCE(): statfs() reads it
  without s_umount while remount can update it.
- Add PREEMPT_RT + remount test data to the commit message.

Changes since v1:
- Replace the sysctl with a mount option named "nostatfs", per review
  (Ard Biesheuvel).

v1: https://lore.kernel.org/all/20260917071600.5587-1-singhpra@juniper.net/
v2: https://lore.kernel.org/all/20260919044124.8268-1-singhpra@juniper.net/

 Documentation/filesystems/efivarfs.rst | 16 +++++++++
 fs/efivarfs/internal.h                 |  1 +
 fs/efivarfs/super.c                    | 46 +++++++++++++++++++++++---
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/efivarfs.rst b/Documentation/filesystems/efivarfs.rst
index f646c3f0980f..cd81ee84115b 100644
--- a/Documentation/filesystems/efivarfs.rst
+++ b/Documentation/filesystems/efivarfs.rst
@@ -37,6 +37,22 @@ accidentally.
           |4_bytes_of_attributes + efivar_data|
           +-----------------------------------+
 
+Mount options
+=============
+
+statfs / nostatfs
+      Control whether ``statfs(2)`` reports the variable-store used/available
+      capacity. Obtaining it requires the ``QueryVariableInfo()`` EFI runtime
+      service, which on some firmware takes tens of milliseconds and runs with
+      preemption disabled, stalling the calling CPU. With ``nostatfs`` the call
+      is skipped and ``statfs(2)`` reports zero. The default is to report,
+      except on ``CONFIG_PREEMPT_RT`` where it defaults to ``nostatfs``; pass
+      ``statfs`` there to force reporting back on (e.g. for tools such as fwupd
+      that need the free space for firmware updates). The option can also be
+      flipped on a live mount with ``mount -o remount,statfs`` /
+      ``mount -o remount,nostatfs``, so reporting can be enabled only for the
+      duration of a firmware update without unmounting efivarfs.
+
 *See also:*
 
 - Documentation/admin-guide/acpi/ssdt-overlays.rst
diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
index f913b6824289..0cb053884b4b 100644
--- a/fs/efivarfs/internal.h
+++ b/fs/efivarfs/internal.h
@@ -11,6 +11,7 @@
 struct efivarfs_mount_opts {
 	kuid_t uid;
 	kgid_t gid;
+	bool nostatfs;		/* skip QueryVariableInfo() in statfs() */
 };
 
 struct efivarfs_fs_info {
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 8d33f11db2a1..d2e1f31a72d1 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -74,6 +74,15 @@ static int efivarfs_show_options(struct seq_file *m, struct dentry *root)
 	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
 		seq_printf(m, ",gid=%u",
 				from_kgid_munged(&init_user_ns, opts->gid));
+	/*
+	 * Always show nostatfs (it makes statfs() report zero capacity, which
+	 * is otherwise surprising); only show statfs where it is not the
+	 * default, i.e. on PREEMPT_RT.
+	 */
+	if (opts->nostatfs)
+		seq_puts(m, ",nostatfs");
+	else if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		seq_puts(m, ",statfs");
 	return 0;
 }
 
@@ -82,13 +91,19 @@ static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	const u32 attr = EFI_VARIABLE_NON_VOLATILE |
 			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			 EFI_VARIABLE_RUNTIME_ACCESS;
+	struct efivarfs_fs_info *sfi = dentry->d_sb->s_fs_info;
 	u64 storage_space, remaining_space, max_variable_size;
 	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
 	efi_status_t status;
 
-	/* Some UEFI firmware does not implement QueryVariableInfo() */
+	/*
+	 * Some UEFI firmware does not implement QueryVariableInfo(); the
+	 * nostatfs mount option also disables this (preempt-disabled,
+	 * potentially slow) call entirely, reporting zero used/available.
+	 */
 	storage_space = remaining_space = 0;
-	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
+	if (!READ_ONCE(sfi->mount_opts.nostatfs) &&
+	    efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
 		static DEFINE_RATELIMIT_STATE(_rs, 2 * HZ, 5);
 		static u64 storage, remaining;
 		static DEFINE_SPINLOCK(lock);
@@ -323,12 +338,13 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
 }
 
 enum {
-	Opt_uid, Opt_gid,
+	Opt_uid, Opt_gid, Opt_statfs,
 };
 
 static const struct fs_parameter_spec efivarfs_parameters[] = {
 	fsparam_uid("uid", Opt_uid),
 	fsparam_gid("gid", Opt_gid),
+	fsparam_flag_no("statfs", Opt_statfs),
 	{},
 };
 
@@ -350,6 +366,9 @@ static int efivarfs_parse_param(struct fs_context *fc, struct fs_parameter *para
 	case Opt_gid:
 		opts->gid = result.gid;
 		break;
+	case Opt_statfs:
+		opts->nostatfs = result.negated;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -402,11 +421,17 @@ static int efivarfs_get_tree(struct fs_context *fc)
 
 static int efivarfs_reconfigure(struct fs_context *fc)
 {
+	struct efivarfs_fs_info *sfi = fc->root->d_sb->s_fs_info;
+	struct efivarfs_fs_info *new_sfi = fc->s_fs_info;
+
 	if (!efivar_supports_writes() && !(fc->sb_flags & SB_RDONLY)) {
 		pr_err("Firmware does not support SetVariableRT. Can not remount with rw\n");
 		return -EINVAL;
 	}
 
+	/* statfs() reads nostatfs locklessly (no s_umount); pair with WRITE_ONCE. */
+	WRITE_ONCE(sfi->mount_opts.nostatfs, new_sfi->mount_opts.nostatfs);
+
 	return 0;
 }
 
@@ -524,8 +549,19 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
 	if (!sfi)
 		return -ENOMEM;
 
-	sfi->mount_opts.uid = GLOBAL_ROOT_UID;
-	sfi->mount_opts.gid = GLOBAL_ROOT_GID;
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		/* Preserve options not respecified on remount. */
+		struct efivarfs_fs_info *old = fc->root->d_sb->s_fs_info;
+
+		sfi->mount_opts.uid = old->mount_opts.uid;
+		sfi->mount_opts.gid = old->mount_opts.gid;
+		sfi->mount_opts.nostatfs = READ_ONCE(old->mount_opts.nostatfs);
+	} else {
+		sfi->mount_opts.uid = GLOBAL_ROOT_UID;
+		sfi->mount_opts.gid = GLOBAL_ROOT_GID;
+		/* QueryVariableInfo() stalls the CPU; default nostatfs on PREEMPT_RT. */
+		sfi->mount_opts.nostatfs = IS_ENABLED(CONFIG_PREEMPT_RT);
+	}
 
 	fc->s_fs_info = sfi;
 	fc->ops = &efivarfs_context_ops;

base-commit: 9b87fdc9af2fbfcdb5c24a64139685ef80f6573f
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] efivarfs: add nostatfs mount option to skip QueryVariableInfo()
  2026-09-25 11:31 [PATCH v3] efivarfs: add nostatfs mount option to skip QueryVariableInfo() Prashant Singh
@ 2026-09-25 12:14 ` Ard Biesheuvel
  2026-09-28 20:21   ` Prashant Singh
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2026-09-25 12:14 UTC (permalink / raw)
  To: Prashant Singh, Jeremy Kerr
  Cc: Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Jonathan Corbet, Shuah Khan, Randy Dunlap,
	Luis Claudio R. Goncalves, Steve McIntyre, linux-efi,
	linux-kernel, linux-doc, linux-rt-devel

Hello Prashant,

Thanks for respinning this.

On Fri, 25 Sep 2026, at 13:31, Prashant Singh wrote:
> QueryVariableInfo() is an EFI runtime service that, on some firmware,
> takes tens of milliseconds and runs with preemption disabled, stalling
> the CPU that services it. efivarfs_statfs() calls it (rate-limited since
> commit b2326338dc68 ("efivarfs: Rate limit statfs() handler")) to report
> the variable-store used/available capacity, so any statfs(2) -- e.g.
> every "df" -- can inject that stall into unrelated latency-sensitive
> workloads on the same CPU.
>
> Add a negatable "nostatfs" mount option: with nostatfs, statfs(2) skips
> QueryVariableInfo() and reports zero used/available; with statfs it
> reports the capacity as before. It defaults to nostatfs on
> CONFIG_PREEMPT_RT so real-time kernels do not take the stall out of the
> box, but statfs can be passed there to force reporting back on -- e.g.
> for tools such as fwupd that need the efivars free space to update Secure
> Boot key databases.
>
> The option can also be toggled on a live mount via remount, so reporting
> can be enabled only for the duration of a firmware update without
> unmounting the boot-time efivarfs mount:
>
>   mount -o remount,statfs   /sys/firmware/efi/efivars   # reporting on
>   mount -o remount,nostatfs /sys/firmware/efi/efivars   # reporting off
>
> Tested on an Intel Xeon E5-2628L v4, 6.12 kernel, via
> "strace -T -e trace=statfs df" on the efivarfs mount.
>
> Cost of the firmware call (CONFIG_PREEMPT_RT not set, so reporting is
> enabled by default). Default mount calls QueryVariableInfo() and returns
> the real store capacity, ~66-129 ms per call:
>
>   statfs("/sys/firmware/efi/efivars", {f_type=EFIVARFS_MAGIC,
>       f_bsize=1, f_blocks=90024, f_bfree=33387, f_bavail=28267, ...})
>       = 0 <0.129124>
>
> With -o nostatfs the firmware call is skipped, capacity reported as
> zero, ~11 us per call:
>
>   statfs("/sys/firmware/efi/efivars", {f_type=EFIVARFS_MAGIC,
>       f_bsize=1, f_blocks=0, f_bfree=0, f_bavail=0, ...}) = 0 <0.000011>
>
> PREEMPT_RT default and live remount toggle (CONFIG_PREEMPT_RT=y). The
> boot-time mount defaults to nostatfs (mount shows nostatfs); no firmware
> call, capacity zero:
>
>   statfs(... f_blocks=0, f_bfree=0, f_bavail=0, ...) = 0 <0.000018>
>
> Enabling reporting in place with "mount -o remount,statfs" (mount now
> shows statfs) takes the ~70 ms firmware call and returns the real
> capacity, without unmounting:
>
>   statfs(... f_blocks=90024, f_bfree=30315, f_bavail=25195, ...)
>       = 0 <0.070293>
>
> Disabling it again with "mount -o remount,nostatfs" returns to the fast,
> zero-capacity path:
>
>   statfs(... f_blocks=0, f_bfree=0, f_bavail=0, ...) = 0 <0.000014>
>
> An unrelated remount that does not respecify the option preserves it: a
> "mount -o remount,rw" after "remount,statfs" leaves the mount showing
> statfs.
>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Prashant Singh <singhpra@juniper.net>
> ---
> Changes since v2:
> - Make the flag negatable (fsparam_flag_no): "statfs" forces reporting
>   back on, "nostatfs" skips QueryVariableInfo(). Default stays nostatfs
>   on PREEMPT_RT. This lets fwupd get the efivars free space it needs for
>   Secure Boot key (KEK/DB/DBX) updates on an RT kernel by mounting with
>   -o statfs (Luis Claudio R. Goncalves, Steve McIntyre).
> - Support toggling the option on a live mount via remount, so reporting
>   can be enabled just for a firmware update without unmounting efivarfs
>   (Sebastian Andrzej Siewior). Options not respecified on remount are
>   preserved.
> - Read/write nostatfs with READ_ONCE()/WRITE_ONCE(): statfs() reads it
>   without s_umount while remount can update it.

Why? There is no memory ordering or tearing issue here, but only a
fundamental race where a remount and a statfs() might occur at the
same time.

> - Add PREEMPT_RT + remount test data to the commit message.
>
> Changes since v1:
> - Replace the sysctl with a mount option named "nostatfs", per review
>   (Ard Biesheuvel).
>
> v1: https://lore.kernel.org/all/20260917071600.5587-1-singhpra@juniper.net/
> v2: https://lore.kernel.org/all/20260919044124.8268-1-singhpra@juniper.net/
>
>  Documentation/filesystems/efivarfs.rst | 16 +++++++++
>  fs/efivarfs/internal.h                 |  1 +
>  fs/efivarfs/super.c                    | 46 +++++++++++++++++++++++---
>  3 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/efivarfs.rst 
> b/Documentation/filesystems/efivarfs.rst
> index f646c3f0980f..cd81ee84115b 100644
> --- a/Documentation/filesystems/efivarfs.rst
> +++ b/Documentation/filesystems/efivarfs.rst
> @@ -37,6 +37,22 @@ accidentally.
>            |4_bytes_of_attributes + efivar_data|
>            +-----------------------------------+
> 
> +Mount options
> +=============
> +
> +statfs / nostatfs
> +      Control whether ``statfs(2)`` reports the variable-store used/available
> +      capacity. Obtaining it requires the ``QueryVariableInfo()`` EFI runtime
> +      service, which on some firmware takes tens of milliseconds and runs with
> +      preemption disabled, stalling the calling CPU. With ``nostatfs`` the call
> +      is skipped and ``statfs(2)`` reports zero. The default is to report,
> +      except on ``CONFIG_PREEMPT_RT`` where it defaults to ``nostatfs``; pass
> +      ``statfs`` there to force reporting back on (e.g. for tools such as fwupd
> +      that need the free space for firmware updates). The option can also be
> +      flipped on a live mount with ``mount -o remount,statfs`` /
> +      ``mount -o remount,nostatfs``, so reporting can be enabled only for the
> +      duration of a firmware update without unmounting efivarfs.
> +
>  *See also:*
> 
>  - Documentation/admin-guide/acpi/ssdt-overlays.rst
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index f913b6824289..0cb053884b4b 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -11,6 +11,7 @@
>  struct efivarfs_mount_opts {
>  	kuid_t uid;
>  	kgid_t gid;
> +	bool nostatfs;		/* skip QueryVariableInfo() in statfs() */
>  };
> 
>  struct efivarfs_fs_info {
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index 8d33f11db2a1..d2e1f31a72d1 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -74,6 +74,15 @@ static int efivarfs_show_options(struct seq_file *m, 
> struct dentry *root)
>  	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
>  		seq_printf(m, ",gid=%u",
>  				from_kgid_munged(&init_user_ns, opts->gid));
> +	/*
> +	 * Always show nostatfs (it makes statfs() report zero capacity, which
> +	 * is otherwise surprising); only show statfs where it is not the
> +	 * default, i.e. on PREEMPT_RT.
> +	 */
> +	if (opts->nostatfs)
> +		seq_puts(m, ",nostatfs");
> +	else if (IS_ENABLED(CONFIG_PREEMPT_RT))
> +		seq_puts(m, ",statfs");

Please drop this else branch - it should be sufficient for the 'nostatfs'
to disappear from the mount options to understand what is going on.

>  	return 0;
>  }
> 
> @@ -82,13 +91,19 @@ static int efivarfs_statfs(struct dentry *dentry, 
> struct kstatfs *buf)
>  	const u32 attr = EFI_VARIABLE_NON_VOLATILE |
>  			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  			 EFI_VARIABLE_RUNTIME_ACCESS;
> +	struct efivarfs_fs_info *sfi = dentry->d_sb->s_fs_info;
>  	u64 storage_space, remaining_space, max_variable_size;
>  	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
>  	efi_status_t status;
> 
> -	/* Some UEFI firmware does not implement QueryVariableInfo() */
> +	/*
> +	 * Some UEFI firmware does not implement QueryVariableInfo(); the
> +	 * nostatfs mount option also disables this (preempt-disabled,
> +	 * potentially slow) call entirely, reporting zero used/available.
> +	 */
>  	storage_space = remaining_space = 0;
> -	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
> +	if (!READ_ONCE(sfi->mount_opts.nostatfs) &&
> +	    efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
>  		static DEFINE_RATELIMIT_STATE(_rs, 2 * HZ, 5);
>  		static u64 storage, remaining;
>  		static DEFINE_SPINLOCK(lock);
> @@ -323,12 +338,13 @@ static int efivarfs_callback(efi_char16_t 
> *name16, efi_guid_t vendor,
>  }
> 
>  enum {
> -	Opt_uid, Opt_gid,
> +	Opt_uid, Opt_gid, Opt_statfs,
>  };
> 
>  static const struct fs_parameter_spec efivarfs_parameters[] = {
>  	fsparam_uid("uid", Opt_uid),
>  	fsparam_gid("gid", Opt_gid),
> +	fsparam_flag_no("statfs", Opt_statfs),
>  	{},
>  };
> 
> @@ -350,6 +366,9 @@ static int efivarfs_parse_param(struct fs_context 
> *fc, struct fs_parameter *para
>  	case Opt_gid:
>  		opts->gid = result.gid;
>  		break;
> +	case Opt_statfs:
> +		opts->nostatfs = result.negated;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -402,11 +421,17 @@ static int efivarfs_get_tree(struct fs_context 
> *fc)
> 
>  static int efivarfs_reconfigure(struct fs_context *fc)
>  {
> +	struct efivarfs_fs_info *sfi = fc->root->d_sb->s_fs_info;
> +	struct efivarfs_fs_info *new_sfi = fc->s_fs_info;
> +
>  	if (!efivar_supports_writes() && !(fc->sb_flags & SB_RDONLY)) {
>  		pr_err("Firmware does not support SetVariableRT. Can not remount with rw\n");
>  		return -EINVAL;
>  	}
> 
> +	/* statfs() reads nostatfs locklessly (no s_umount); pair with WRITE_ONCE. */
> +	WRITE_ONCE(sfi->mount_opts.nostatfs, new_sfi->mount_opts.nostatfs);
> +
>  	return 0;
>  }
> 
> @@ -524,8 +549,19 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
>  	if (!sfi)
>  		return -ENOMEM;
> 
> -	sfi->mount_opts.uid = GLOBAL_ROOT_UID;
> -	sfi->mount_opts.gid = GLOBAL_ROOT_GID;
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +		/* Preserve options not respecified on remount. */
> +		struct efivarfs_fs_info *old = fc->root->d_sb->s_fs_info;
> +
> +		sfi->mount_opts.uid = old->mount_opts.uid;
> +		sfi->mount_opts.gid = old->mount_opts.gid;
> +		sfi->mount_opts.nostatfs = READ_ONCE(old->mount_opts.nostatfs);
> +	} else {
> +		sfi->mount_opts.uid = GLOBAL_ROOT_UID;
> +		sfi->mount_opts.gid = GLOBAL_ROOT_GID;
> +		/* QueryVariableInfo() stalls the CPU; default nostatfs on PREEMPT_RT. */
> +		sfi->mount_opts.nostatfs = IS_ENABLED(CONFIG_PREEMPT_RT);
> +	}
> 

Why do we need this? Does remount otherwise forget its options? Does this
mean that before this patch, remounting with only a different guid would
reset the uid specified at mount time to the default? If so, please fix
that in a separate (preceding) patch.

 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] efivarfs: add nostatfs mount option to skip QueryVariableInfo()
  2026-09-25 12:14 ` Ard Biesheuvel
@ 2026-09-28 20:21   ` Prashant Singh
  0 siblings, 0 replies; 3+ messages in thread
From: Prashant Singh @ 2026-09-28 20:21 UTC (permalink / raw)
  To: ardb, jk
  Cc: Prashant Singh, Sebastian Andrzej Siewior, Clark Williams,
	Steven Rostedt, Jonathan Corbet, Shuah Khan, Randy Dunlap,
	Luis Claudio R . Goncalves, Steve McIntyre, linux-efi,
	linux-kernel, linux-doc, linux-rt-devel

Hi Ard,

Thanks for the review. v4 addresses all three points:

On Fri, 25 Sep 2026, at 14:14, Ard Biesheuvel wrote:
> > - Read/write nostatfs with READ_ONCE()/WRITE_ONCE(): statfs() reads it
> >   without s_umount while remount can update it.
>
> Why? There is no memory ordering or tearing issue here, but only a
> fundamental race where a remount and a statfs() might occur at the
> same time.

That's right -- a concurrent remount and statfs() is the only case, and
it's benign (no ordering or tearing for a bool). I've dropped that
changelog bullet. I kept READ_ONCE()/WRITE_ONCE() only to mark the
intentional unlocked access for KCSAN, but I'm happy to drop them if you'd
prefer.

> > +	if (opts->nostatfs)
> > +		seq_puts(m, ",nostatfs");
> > +	else if (IS_ENABLED(CONFIG_PREEMPT_RT))
> > +		seq_puts(m, ",statfs");
>
> Please drop this else branch - it should be sufficient for the 'nostatfs'
> to disappear from the mount options to understand what is going on.

Dropped in v4. Only ",nostatfs" is shown; its absence means reporting is on.

> > +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> > +		sfi->mount_opts.uid = old->mount_opts.uid;
> > +		sfi->mount_opts.gid = old->mount_opts.gid;
>
> Why do we need this? Does remount otherwise forget its options? Does this
> mean that before this patch, remounting with only a different gid would
> reset the uid specified at mount time to the default? If so, please fix
> that in a separate (preceding) patch.

No reset bug. efivarfs_reconfigure() only writes nostatfs back to the live
sb, never uid/gid, so a remount leaves uid/gid at their mount-time values
(verified on a PREEMPT_RT box: uid=1000 survives "remount,gid=2000", gid
ignored). The uid/gid copies were therefore dead code -- removed. The
branch now only preserves nostatfs, the one option remount can actually
change.

Thanks,
Prashant

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-28 20:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-25 11:31 [PATCH v3] efivarfs: add nostatfs mount option to skip QueryVariableInfo() Prashant Singh
2026-09-25 12:14 ` Ard Biesheuvel
2026-09-28 20:21   ` Prashant Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox