mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Prashant Singh <singhpra@juniper.net>
To: <jk@ozlabs.org>, <ardb@kernel.org>
Cc: <linux-efi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<corbet@lwn.net>, <linux-doc@vger.kernel.org>,
	Prashant Singh <singhpra@juniper.net>
Subject: [PATCH] efivarfs: avoid slow QueryVariableInfo() in statfs()
Date: Thu, 17 Sep 2026 00:16:00 -0700	[thread overview]
Message-ID: <20260917071600.5587-1-singhpra@juniper.net> (raw)

QueryVariableInfo() is an EFI runtime service that, on some platforms,
takes tens of milliseconds and runs with preemption disabled, freezing
the CPU that services it for the whole call. efivarfs_statfs() issued
this call on every statfs(2) (e.g. every "df"), which produced large
latency spikes for unrelated latency-sensitive workloads pinned to the
same CPU.

Commit b2326338dc68 ("efivarfs: Rate limit statfs() handler") already
bounds the firmware call to twice per second. That helps against a
statfs() flood, but on a real-time / dataplane system even the residual
2 calls/s x ~40ms = up to ~80ms/s of preempt-disabled CPU time is
unacceptable: any unprivileged process (or a periodic monitoring "df")
can still inject ~40ms stalls into a co-located latency-sensitive task.

Cache the reported capacity instead of querying firmware from the
statfs() hot path:

  - The cache is primed once at mount time so df has valid numbers.
  - When fs.efivarfs.statfs_refresh is set (default 1, preserving the
    current rate-limited behaviour), statfs() refreshes the cache in the
    same rate-limited way.
  - Latency-sensitive systems can set fs.efivarfs.statfs_refresh to 0 so
    statfs() never calls firmware; the cache is then kept up to date only
    by mount and explicit fs.efivarfs.force_refresh writes.
  - fs.efivarfs.force_refresh is a write-only knob that forces an
    immediate synchronous re-query on demand.

On a hard query failure (a status other than EFI_SUCCESS or
EFI_UNSUPPORTED) the last known-good cached value is retained rather than
overwritten, so a transient firmware error does not zero out the
reported capacity.

Measured on an Intel Xeon E5-2628L v4 (6.12 kernel), per statfs(2) on the
efivarfs mount:

  # fs.efivarfs.statfs_refresh = 1 (firmware call on the hot path)
  $ strace -T -e trace=statfs df 2>&1 | grep -i efivars
  statfs("/sys/firmware/efi/efivars", {...}) = 0 <0.062993>
  statfs("/sys/firmware/efi/efivars", {...}) = 0 <0.064343>

  # after: sysctl fs.efivarfs.statfs_refresh=0 (served from cache)
  $ strace -T -e trace=statfs df 2>&1 | grep -i efivars
  statfs("/sys/firmware/efi/efivars", {...}) = 0 <0.000009>

  # in a tight df loop with refresh=1, rate-limited hits show the cache
  # (sub-ms) interleaved with the ~63ms firmware calls:
  statfs(...) = 0 <0.063705>
  statfs(...) = 0 <0.000429>
  statfs(...) = 0 <0.000079>

force_refresh picks up an intervening variable write on demand without
re-enabling the hot-path call:

  $ df -h | grep efivars
  efivarfs   88K  54K  30K  65% /sys/firmware/efi/efivars
  $ sysctl -w fs.efivarfs.force_refresh=1
  $ df -h | grep efivars
  efivarfs   88K  55K  29K  66% /sys/firmware/efi/efivars

Signed-off-by: Prashant Singh <singhpra@juniper.net>
---
An alternative would be to expose statfs_refresh as an efivarfs mount
option instead of a global sysctl (per-mount, no new sysctl ABI, and a
remount could re-prime the cache in place of force_refresh). I went with
the sysctl for a live runtime toggle and because the variable store /
cache are inherently global, but I'm happy to respin as a mount option
if that's preferred.

 Documentation/admin-guide/sysctl/fs.rst |  24 +++++
 fs/efivarfs/super.c                     | 132 +++++++++++++++++++-----
 2 files changed, 128 insertions(+), 28 deletions(-)

diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
index 9b7f65c3efd8..4b6683b6791c 100644
--- a/Documentation/admin-guide/sysctl/fs.rst
+++ b/Documentation/admin-guide/sysctl/fs.rst
@@ -76,6 +76,30 @@ they help speeding up rejection of non-existing files provided
 by the users.
 
 
+efivarfs
+--------
+
+These entries appear under ``/proc/sys/fs/efivarfs`` and control how
+``statfs(2)`` on an efivarfs mount reports the EFI variable-store
+capacity. The capacity is obtained from the ``QueryVariableInfo()`` EFI
+runtime service, which on some x86 platforms can take tens of
+milliseconds. To avoid this cost on the ``statfs(2)`` hot path the
+reported capacity is cached; these knobs control how the cache is
+refreshed.
+
+``statfs_refresh`` (default 1)
+  When set to 1, ``statfs(2)`` refreshes the cached capacity by calling
+  the firmware, rate-limited to a few calls every couple of seconds.
+  When set to 0, ``statfs(2)`` never calls firmware and always serves the
+  cached value; the cache is then updated only at mount time and by
+  ``force_refresh``. Set this to 0 on latency-sensitive systems.
+
+``force_refresh`` (write-only)
+  Writing any value forces an immediate, synchronous re-query of the
+  firmware to update the cache. Useful to pick up capacity changes when
+  ``statfs_refresh`` is 0. The written value is ignored.
+
+
 file-max & file-nr
 ------------------
 
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 8d33f11db2a1..1e7e81766c28 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -19,6 +19,7 @@
 #include <linux/notifier.h>
 #include <linux/printk.h>
 #include <linux/namei.h>
+#include <linux/sysctl.h>
 
 #include "internal.h"
 #include "../internal.h"
@@ -77,44 +78,71 @@ static int efivarfs_show_options(struct seq_file *m, struct dentry *root)
 	return 0;
 }
 
-static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
+/*
+ * Cached EFI variable-store capacity: QueryVariableInfo() is costly on some
+ * firmware, so statfs() serves this cache instead of calling firmware.
+ */
+static DEFINE_SPINLOCK(efivarfs_cap_lock);
+static u64 efivarfs_cap_storage;
+static u64 efivarfs_cap_remaining;
+
+/*
+ * fs.efivarfs.statfs_refresh (default 1): when set, statfs() refreshes the
+ * cached capacity, rate-limited. Set to 0 on latency-sensitive systems so
+ * statfs() never calls firmware; the cache is then updated only at mount and by
+ * fs.efivarfs.force_refresh.
+ */
+static int efivarfs_statfs_refresh = 1;
+
+static void efivarfs_query_capacity(void)
 {
 	const u32 attr = EFI_VARIABLE_NON_VOLATILE |
 			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			 EFI_VARIABLE_RUNTIME_ACCESS;
-	u64 storage_space, remaining_space, max_variable_size;
-	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
+	u64 storage_space = 0, remaining_space = 0, max_variable_size;
 	efi_status_t status;
 
-	/* Some UEFI firmware does not implement QueryVariableInfo() */
-	storage_space = remaining_space = 0;
-	if (efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO)) {
+	if (!efivar_is_available() ||
+	    !efi_rt_services_supported(EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO))
+		return;
+
+	status = efivar_query_variable_info(attr, &storage_space,
+					    &remaining_space, &max_variable_size);
+	if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED) {
+		pr_warn_ratelimited("query_variable_info() failed: 0x%lx\n", status);
+		return;
+	}
+
+	spin_lock(&efivarfs_cap_lock);
+	efivarfs_cap_storage = storage_space;
+	efivarfs_cap_remaining = remaining_space;
+	spin_unlock(&efivarfs_cap_lock);
+}
+
+static int efivarfs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+	u64 id = huge_encode_dev(dentry->d_sb->s_dev);
+	u64 storage_space, remaining_space;
+
+	/*
+	 * Refresh the cached capacity (rate-limited) unless statfs_refresh is
+	 * disabled, in which case statfs() serves the cache without ever calling
+	 * the expensive QueryVariableInfo() firmware service.
+	 */
+	if (READ_ONCE(efivarfs_statfs_refresh)) {
 		static DEFINE_RATELIMIT_STATE(_rs, 2 * HZ, 5);
-		static u64 storage, remaining;
-		static DEFINE_SPINLOCK(lock);
 
-		if (!__ratelimit(&_rs)) {
+		if (__ratelimit(&_rs))
+			efivarfs_query_capacity();
+		else
 			ratelimit_set_flags(&_rs, RATELIMIT_MSG_ON_RELEASE);
-
-			spin_lock(&lock);
-			storage_space = storage;
-			remaining_space = remaining;
-			spin_unlock(&lock);
-		} else {
-			status = efivar_query_variable_info(attr, &storage_space,
-							    &remaining_space,
-							    &max_variable_size);
-			if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED)
-				pr_warn("query_variable_info() failed: 0x%lx\n",
-					status);
-
-			spin_lock(&lock);
-			storage = storage_space;
-			remaining = remaining_space;
-			spin_unlock(&lock);
-		}
 	}
 
+	spin_lock(&efivarfs_cap_lock);
+	storage_space = efivarfs_cap_storage;
+	remaining_space = efivarfs_cap_remaining;
+	spin_unlock(&efivarfs_cap_lock);
+
 	/*
 	 * This is not a normal filesystem, so no point in pretending it has a block
 	 * size; we declare f_bsize to 1, so that we can then report the exact value
@@ -392,6 +420,9 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		return err;
 
+	/* Prime the capacity cache once at mount so df has valid numbers. */
+	efivarfs_query_capacity();
+
 	return efivar_init(efivarfs_callback, sb, true);
 }
 
@@ -552,13 +583,58 @@ static struct file_system_type efivarfs_type = {
 	.fs_flags = FS_POWER_FREEZE,
 };
 
+static int efivarfs_force_refresh;
+
+static int efivarfs_force_refresh_handler(const struct ctl_table *table, int write,
+					  void *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret = proc_dointvec(table, write, buffer, lenp, ppos);
+
+	/* The written value is irrelevant; any write triggers a re-query. */
+	if (!ret && write)
+		efivarfs_query_capacity();
+	return ret;
+}
+
+static const struct ctl_table efivarfs_sysctls[] = {
+	{
+		.procname	= "statfs_refresh",
+		.data		= &efivarfs_statfs_refresh,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
+	{
+		.procname	= "force_refresh",
+		.data		= &efivarfs_force_refresh,
+		.maxlen		= sizeof(int),
+		.mode		= 0200,
+		.proc_handler	= efivarfs_force_refresh_handler,
+	},
+};
+
+static struct ctl_table_header *efivarfs_sysctl_header;
+
 static __init int efivarfs_init(void)
 {
-	return register_filesystem(&efivarfs_type);
+	int err = register_filesystem(&efivarfs_type);
+
+	if (err)
+		return err;
+
+	efivarfs_sysctl_header = register_sysctl("fs/efivarfs", efivarfs_sysctls);
+	if (!efivarfs_sysctl_header)
+		pr_warn("efivarfs: unable to register sysctls\n");
+
+	return 0;
 }
 
 static __exit void efivarfs_exit(void)
 {
+	if (efivarfs_sysctl_header)
+		unregister_sysctl_table(efivarfs_sysctl_header);
 	unregister_filesystem(&efivarfs_type);
 }
 
-- 
2.34.1

base-commit: 9b87fdc9af2fbfcdb5c24a64139685ef80f6573f

             reply	other threads:[~2026-09-17  7:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  7:16 Prashant Singh [this message]
2026-09-17  8:36 ` Ard Biesheuvel
2026-09-17 16:50   ` Prashant Singh

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=20260917071600.5587-1-singhpra@juniper.net \
    --to=singhpra@juniper.net \
    --cc=ardb@kernel.org \
    --cc=corbet@lwn.net \
    --cc=jk@ozlabs.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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®