[PATCH v2 22/50] convert efivarfs

[Ard Biesheuvel]

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.


I did need to apply a fixup to get the revert to compile:

--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -412,8 +412,7 @@
 {
        unsigned long size;
        struct efivarfs_ctx *ectx = container_of(ctx, struct efivarfs_ctx, ctx);
-       struct qstr qstr = { .name = name, .len = len };
-       struct dentry *dentry = d_hash_and_lookup(ectx->sb->s_root, &qstr);
+       struct dentry *dentry = try_lookup_noperm(&QSTR_LEN(name,
len), ectx->sb->s_root);
        struct inode *inode;
        struct efivar_entry *entry;
        int err;