Re: [PATCH v4] efivarfs: add nostatfs mount option to skip QueryVariableInfo()

[MaeeFilho FL]

Hi Prashant, Ard, Sebastian,

Regarding the concurrent update validation on efivarfs_reconfigure() and the
KCSAN annotations discussed for the benign data race:

The implementation of data_race() directly wrapping the assignment of the
nostatfs flag inside the reconfiguration path is clean and highly efficient
for low-level standalone execution boundaries:

    data_race(sfi->mount_opts.nostatfs = new_sfi->mount_opts.nostatfs);

Since the reads inside efivarfs_statfs() and efivarfs_show_options() stay plain,
this approach perfectly satisfies runtime instrumentation checks without adding
dead memory barriers or locking overhead to unprivileged statfs(2)/df routines.

Furthermore, preventing the global CPU stall caused by the SMM rendezvous during
QueryVariableInfo() on real-time environments (CONFIG_PREEMPT_RT) by defaulting
to nostatfs is a major architectural improvement for deterministic scheduling.

For the final v5 patch spin, keeping the commit log focused on the preemption
disabled context and dropping the verbose raw strace outputs as suggested
makes the patch documentation extremely tight and industrial grade.

Acked-by: dev12124 (João Guilherme da Silva Freitas Lima)
<joaoanandalima@xxxxxxxxx>

Em dom., 4 de out. de 2026 às 04:59, Ard Biesheuvel <ardb@xxxxxxxxxx> escreveu:
>
>
> On Thu, 1 Oct 2026, at 03:20, Prashant Singh wrote:
> > Thanks Ard and Sebastian for the comments.
> >
> ...
> >>AFAICT, that would potentially leave KCSAN instrumentation on the reboot
> >>path, which might trigger and interfere with the reboot. So instead,
> >>I'd like to put this in efi_reboot_required if we can. If it is needed
> >>in more places to address an actual KCSAN splat, I don't mind. If it is
> >>just to make Sashiko happy, then we shouldn't bother.
> >
> > Could you please clarify what you mean by efi_reboot_required here? nostatfs
> > is only read in efivarfs_statfs(), efivarfs_show_options() and
> > efivarfs_init_fs_context(), none of which run on the reboot path, so I'm
> > not sure how it would apply.
> >
>
> Apologies, I managed to completely confuse myself here. Forget what I said
> here, please :-)
>
> > On the annotation itself: an internal review flagged a potential KCSAN
> > data race rather than an observed splat -- statfs() can run concurrently
> > with a remount updating the flag, so it is a genuine (benign) concurrent
> > access. I ran concurrent statfs/remount loops on separate CPUs under
> > KCSAN and didn't trigger a report in a bounded run, which could be
> > expected given KCSAN samples accesses, so it doesn't disprove the race.
> > Since KCSAN only needs one side of the pair marked, data_race() on the
> > write covers both readers and the reads stay plain. I'm happy to drop it
> > entirely if you'd prefer to keep the benign race unannotated.
> >
>
> No, let's keep it as you suggest.
>
> >>Please keep this description _here_ where you have it. Once this is
> >>merged, you could send another patch, extending the documentation with
> >>the statfs option (I think the workqueue change is in).
> >
> > Sure -- I'll keep it in Documentation/filesystems/efivarfs.rst for now
> > and send a follow-up extending Documentation/core-api/real-time/hardware.rst
> > once this is merged.
> >
> >>You still have the problem that someone reading the variable leads to
> >>the same problem but this requires a privileged user. And if I am not
> >>mistaken, someone sent patches to have efi-runtime runtime disabled/
> >>enabled.
> >
> > Agreed -- the variable-read path is the same, but needs a privileged
> > user unlike the unprivileged statfs()/df trigger.
> >
>
> Indeed - this is only about anyone with read permissions on the mount
> point being able to trigger this. And looking at your results, the
> rate limit we added recently might be a bit too permissive as well.
>