On Mon, 2025-03-31 at 14:42 +0200, Christian Brauner wrote:
> Allow efivarfs to partake to resync variable state during system
> hibernation and suspend. Add freeze/thaw support.
>
> This is a pretty straightforward implementation. We simply add
> regular freeze/thaw support for both userspace and the kernel. This
> works without any big issues and congrats afaict efivars is the first
> pseudofilesystem that adds support for filesystem freezing and
> thawing.
>
> The simplicity comes from the fact that we simply always resync
> variable state after efivarfs has been frozen. It doesn't matter
> whether that's because of suspend, userspace initiated freeze or
> hibernation. Efivars is simple enough that it doesn't matter that we
> walk all dentries. There are no directories and there aren't insane
> amounts of entries and both freeze/thaw are already heavy-handed
> operations. We really really don't need to care.
Just as a point of order: this can't actually work until freeze/thaw is
actually plumbed into suspend/resume.
>
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> ---
> fs/efivarfs/internal.h | 1 -
> fs/efivarfs/super.c | 196 +++++++++++++--------------------------
> ----------
> 2 files changed, 51 insertions(+), 146 deletions(-)
>
> diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h
> index ac6a1dd0a6a5..f913b6824289 100644
> --- a/fs/efivarfs/internal.h
> +++ b/fs/efivarfs/internal.h
> @@ -17,7 +17,6 @@ struct efivarfs_fs_info {
> struct efivarfs_mount_opts mount_opts;
> struct super_block *sb;
> struct notifier_block nb;
> - struct notifier_block pm_nb;
> };
>
> struct efi_variable {
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index 0486e9b68bc6..567e849a03fe 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -20,6 +20,7 @@
> #include <linux/printk.h>
>
> #include "internal.h"
> +#include "../internal.h"
>
> static int efivarfs_ops_notifier(struct notifier_block *nb, unsigned
> long event,
> void *data)
> @@ -119,12 +120,18 @@ static int efivarfs_statfs(struct dentry
> *dentry, struct kstatfs *buf)
>
> return 0;
> }
> +
> +static int efivarfs_freeze_fs(struct super_block *sb);
> +static int efivarfs_unfreeze_fs(struct super_block *sb);
> +
> static const struct super_operations efivarfs_ops = {
> .statfs = efivarfs_statfs,
> .drop_inode = generic_delete_inode,
> .alloc_inode = efivarfs_alloc_inode,
> .free_inode = efivarfs_free_inode,
> .show_options = efivarfs_show_options,
> + .freeze_fs = efivarfs_freeze_fs,
Why is it necessary to have a freeze_fs operation? The current code in
super.c:freeze_super() reads:
if (sb->s_op->freeze_fs) {
ret = sb->s_op->freeze_fs(sb);
So it would seem that setting this to NULL has exactly the same effect
as providing a null method.
> + .unfreeze_fs = efivarfs_unfreeze_fs,
> };
>
> /*
>
[...]
> @@ -608,9 +516,7 @@ static void efivarfs_kill_sb(struct super_block
> *sb)
> {
> struct efivarfs_fs_info *sfi = sb->s_fs_info;
>
> - blocking_notifier_chain_unregister(&efivar_ops_nh, &sfi-
> >nb);
This is an extraneous deletion of an unrelated notifier which efivarfs
still needs to listen for ops updates from the efi subsystem.
Regards,
James
