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

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 30 Sep 2026, at 03:35, Prashant Singh wrote:
> Thanks Ard and Sebastian for the comments.
>
>>Please don't respond to questions by respinning the patch without
>>having any discussion at all.
>
> Noted, and apologies for sending v4 without first finalizing the v3
> discussion.
>
>>I asked you an honest question, and it seems Sashiko spotted an
>>issue here too.
>>Is there a problem with how the current code deals with uid and gid
>>on a remount?
>
> No, I checked and did not find a problem. I mentioned this in my
> response to your query on v3:
> https://lore.kernel.org/all/20260925113145.7396-1-singhpra@juniper.net/
>

Apologies, I missed that.

>>This until the end looks extremely verbose.
>
> I can drop the strace data from the commit message if that reads
> cleaner; I had included it to highlight the issue.
>

Yeah you can trim it a bit but git commit log space is cheap and
only people that care about the patch will read it anyway.

As for the docs and code comment changes themselves, please make
those a bit more to the point. People can go and find the commit
that introduced/updated them to know more about the background.

>>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,

might block all CPUs for a few milliseconds.

>>                        reporting 0 for used and capacity. Enabled by default on

Instead, report 0/0 for used/available.

>>                        PREEMPT_RT.
>>===================     =========================================================
>>
>>It might make sense to add this knob to Documentation/core-api/real-time/hardware.rst.
>
> Thanks, I will use this wording and move it to
> Documentation/core-api/real-time/hardware.rst.
>
>>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.
>
> Apologies, that was a miss on my part, and I saw that sashiko-bot
> flagged it too. I wanted to gather any additional comments before
> addressing it.
>
>>Yeah the most severe issue is that entering SMM requires a rendez-vous
>>of all the cores, and so whether preemption is enabled or not is
>>actually kind of irrelevant, given that all the other cores just
>>disappear.
>
> Yes, I observed the same behavior. I was not sure whether this is
> x86-specific, so I refrained from mentioning it in the commit log.
>

It is x86 specific but that is the main reason we need any of this.

>>Right. So if anything, there could be a data_race() in one place
>>instead inconsistent READ/ WRITE once as we have it now.
>
> I would use data_race() on the write for nostatfs. Please let me know
> if the below looks OK:
>
>     /* nostatfs is read locklessly by statfs()/show_options(); the racy 
> update is benign. */
>     data_race(sfi->mount_opts.nostatfs = new_sfi->mount_opts.nostatfs);
>

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.