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

[Prashant Singh <singhpra@juniper.net>]

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/

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

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

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.

>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);

Thanks,
Prashant