[PATCH v2 22/50] convert efivarfs

[Christian Brauner]

On Thu, Oct 30, 2025 at 02:35:51PM +0100, Ard Biesheuvel wrote:
> On Wed, 29 Oct 2025 at 20:38, Al Viro <viro at zeniv.linux.org.uk> wrote:
> >
> > On Wed, Oct 29, 2025 at 02:57:51PM -0400, James Bottomley wrote:
> >
> > > I think this all looks OK.  The reason for the convolution is that
> > > simple_start/done_creating() didn't exist when I did the conversion ...
> > > although if they had, I'm not sure I'd have thought of reworking
> > > efivarfs_create_dentry to use them.  I tried to update some redundant
> > > bits, but it wasn't the focus of what I was trying to fix.
> > >
> > > So I think the cleanup works and looks nice.
> > >
> > > >
> > > > Relying on the -EEXIST return value to detect duplicates, and
> > > > combining the two callbacks seem like neat optimizations to me, so
> > > >
> > > > Acked-by: Ard Biesheuvel <ardb at kernel.org>
> > > >
> > > > but I have to confess I am slightly out of my depth when it comes to
> > > > VFS stuff.
> > >
> > > Yes, ack too.
> >
> >         Umm...  FWIW, I've got a few more followups on top of that (see
> > #untested.efivarfs, current head at 36051c773015).  Not sure what would
> > be the best way to deal with that stuff - I hope to get the main series
> > stabilized and merged in the coming window.  Right now I'm collecting
> > feedback (acked-by, etc.), and there's a couple of outright bugfixes
> > in front of the series, so I'd expect at least a rebase to -rc4...
> >
> 
> I pulled your code and tried to test it. It works fine for the
> ordinary case, but only now I realized that commit
> 
> commit 0e4f9483959b785f65a36120bb0e4cf1407e492c
> Author: Christian Brauner <brauner at kernel.org>
> Date:   Mon Mar 31 14:42:12 2025 +0200
> 
>     efivarfs: support freeze/thaw
> 
> actually broke James's implementation of the post-resume sync with the
> underlying variable store.
> 
> So I wonder what the point is of all this complexity if it does not
> work for the use case where it is the most important, i.e., resume
> from hibernation, where the system goes through an ordinary cold boot
> and so the EFI variable store may have gotten out of sync with the
> hibernated kernel's view of it.
> 
> If no freeze/thaw support in the suspend/resume path is forthcoming,
> would it be better to just revert that change? That would badly
> conflict with your changes, though, so I'd like to resolve this before
> going further down this path.

So first of all, this works. I've tested it extensively. If it doesn't
work there's a regression.

And suspend/resume works just fine with freeze/thaw. See commit
eacfbf74196f ("power: freeze filesystems during suspend/resume") which
implements exactly that.

The reason this didn't work for you is very likely:

cat /sys/power/freeze_filesystems
0

which you must set to 1.

Second, that "complexity" replaces your way more complex blocking
notifier implementation for this thing which simply deadlocked the
system as I reported and showed earlier this year.

That blocking notifier thing had to use vfs_kern_mount() which had to
come up with it's own internal private vfs mount to pin efivarfs because
it's called out-of-band and then walk the list of variables and resync.
Problem is that leads to completely untenable locking problems. So if
you want to go back to that be my guest.