Re: [PATCH v2 22/50] convert efivarfs

Christian Brauner
2025-11-05T15:23:46Z

On Wed, Nov 05, 2025 at 09:01:59AM -0500, James Bottomley wrote:
> On Wed, 2025-11-05 at 14:46 +0100, Christian Brauner wrote:
> > On Wed, Nov 05, 2025 at 08:33:10AM -0500, James Bottomley wrote:
> > > On Wed, 2025-11-05 at 14:16 +0100, Christian Brauner wrote:
> > > > On Wed, Nov 05, 2025 at 08:09:03AM -0500, James Bottomley wrote:
> > > > > On Wed, 2025-11-05 at 12:47 +0100, Christian Brauner wrote:
> > > [...]
> > > > > > 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.
> > > > >
> > > > > Actually, no, that's not correct.  The efivarfs freeze/thaw
> > > > > logic must run unconditionally regardless of this setting to
> > > > > fix the systemd bug, so all the variable resyncing is done in
> > > > > the thaw call, which isn't conditioned on the above (or at
> > > > > least it shouldn't be).
> > > >
> > > > It is conditioned on the above currently but we can certainly fix
> > > > it easily to not be.
> > >
> > > It still seems to be unconditional in upstream 6.18-rc4
> > > kernel/power/hibernate.c with only freeze being conditioned on the
> >
> > I'm honestly not sure how efivarfs would be frozen if
> > filesystems_freeze() isn't called... Maybe I missed that memo though.
> > In any case I just sent you...
>
> We don't need to be frozen: our freeze_fs method is empty, we just need
> thaw_fs calling.

No, you need to call freeze so the power subsystem can mark the
filesystem as being exclusively frozen by it because that specific
freeze must not be undone by anyone else e.g., userspace or some other
internal unfreeze due to some filesystem (for other filesystems this is
very relevant) internal freeze for say scrub or whatever.

If filesystem_thaw() doesn't find efivarfs frozen - and exclusively
frozen by the power subsyste - it obviously won't call the actual
efivarfs thaw method. It's all working in order. My patch should fix
your issue and will ensure efivarfs always runs. We wouldn't even need
an SB_I_* flag for this. We could equally well just match superblock but
other filesystems might need or want to opt into this too.

Don't implement thaw_super() yourself, please.

>
> Is the trouble that there's now freeze/thaw accounting, so thaw won't
> be called based on that if freeze wasn't? In which case might it not
> be better for us to implement thaw_super, which is called
> unconditionally and leaves the accounting up to the filesystem?
>
> > > setting of the filesystem_freeze variable but I haven't checked -
> > > next.
> > >
> > > However, if there's anything in the works to change that we would
> > > need an exception for efivarfs, please ... we can't have a bug fix
> > > conditioned on a user setting.
> >
> > ... a patch in another mail.
> >
> > Sorry in case I misunderstood that you _always_ wanted that sync
> > regardless of userspace enabling it.
>
> We need the thaw method called to get the variable resync to happen.
> That fixes a bug on hibernate with systemd (and also accounts for an
> other efi variable changes the user may have made between hibernate and
> resume), yes. And we need that to happen unconditionally to fix the
> systemd bug.
>
> Regards,
>
> James
>