mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Prashant Singh <singhpra@juniper.net>
Cc: ardb@kernel.org, jk@ozlabs.org, clrkwllms@kernel.org,
	rostedt@goodmis.org, corbet@lwn.net, skhan@linuxfoundation.org,
	rdunlap@infradead.org, lgoncalv@redhat.com, 93sam@debian.org,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-rt-devel@lists.linux.dev
Subject: Re: [PATCH v4] efivarfs: add nostatfs mount option to skip QueryVariableInfo()
Date: Tue, 29 Sep 2026 09:43:12 +0200	[thread overview]
Message-ID: <20260929074312.v3jT8uel@linutronix.de> (raw)
In-Reply-To: <20260928203445.72318-1-singhpra@juniper.net>

On 2026-09-28 13:34:45 [-0700], 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.

This until the end looks extremely verbose. Even if that statfs takes
ages, that important part is that it does so with disables preemption.
There has been also the introduction of the efi_runtime workqueue which
can be pinned to a single CPU. I guess this doesn't work for you or the
EFI firmware takes all other CPUs down until the all completes.

> 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: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Prashant Singh <singhpra@juniper.net>
> ---
> Changes since v3:
> - Drop the show_options "statfs" branch; the absence of "nostatfs" now
>   indicates reporting is on (Ard Biesheuvel).
> - Drop the uid/gid copies on the remount path; efivarfs_reconfigure()
>   applies only nostatfs, so they were dead code (Ard Biesheuvel).
> 
> 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.
> - 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/
> v3: https://lore.kernel.org/all/20260925113145.7396-1-singhpra@juniper.net/
> 
>  Documentation/filesystems/efivarfs.rst | 16 +++++++++++
>  fs/efivarfs/internal.h                 |  1 +
>  fs/efivarfs/super.c                    | 38 ++++++++++++++++++++++----
>  3 files changed, 50 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.

could this be, I don't know something smaller not including the
commandline where I would expect that people know how to use it.

===================     =========================================================
(no)statfs		Control whether ``statfs(2)`` reports the variable-store
			used/ available. Disabling it skips the EFI runtime 
			service call, which might block the CPU for a few milliseconds,
			reporting 0 for used and capacity. Enabled by default on
			PREEMPT_RT.
===================     =========================================================

It might make sense to add this knob to Documentation/core-api/real-time/hardware.rst.
I am not sure yet but slowly we are getting more knobs that I have
expected.

> +
>  *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() */
Here and below you add a comment to every change you make. What about
focusing on the important parts, that deserve an explanation why a
change has been made. For instance why nostatfs has the READ_ONCE/
WRITE_ONCE accessors and sometimes it does not.

>  };
>  

Sebastian

  parent reply	other threads:[~2026-09-29  7:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 20:34 Prashant Singh
2026-09-28 20:47 ` sashiko-bot
2026-09-29  6:32 ` Ard Biesheuvel
2026-09-29  7:43 ` Sebastian Andrzej Siewior [this message]
2026-09-29 12:22   ` Ard Biesheuvel
2026-09-29 15:14     ` Sebastian Andrzej Siewior

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=20260929074312.v3jT8uel@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=93sam@debian.org \
    --cc=ardb@kernel.org \
    --cc=clrkwllms@kernel.org \
    --cc=corbet@lwn.net \
    --cc=jk@ozlabs.org \
    --cc=lgoncalv@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=singhpra@juniper.net \
    --cc=skhan@linuxfoundation.org \
    /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

all inboxes | Powered by JetHome®