Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20250329-work-freeze-v2-6-a47af37ecc3d@kernel.org>
References: <20250329-work-freeze-v2-6-a47af37ecc3d@kernel.org>
TO: Christian Brauner <brauner@kernel.org>

Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on acb4f33713b9f6cadb6143f211714c343465411c]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Brauner/super-remove-pointless-s_root-checks/20250329-164716
base:   acb4f33713b9f6cadb6143f211714c343465411c
patch link:    https://lore.kernel.org/r/20250329-work-freeze-v2-6-a47af37ecc3d%40kernel.org
patch subject: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate
:::::: branch date: 21 hours ago
:::::: commit date: 21 hours ago
config: x86_64-randconfig-161-20250330 (https://download.01.org/0day-ci/archive/20250330/202503301305.wiPCA3TP-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202503301305.wiPCA3TP-lkp@intel.com/

smatch warnings:
fs/super.c:1187 get_active_super() error: uninitialized symbol 'active'.

vim +/active +1187 fs/super.c

08fdc8a0138afa Mateusz Guzik     2017-10-03  1178  
1bd87d01c81377 Christian Brauner 2025-03-29  1179  static inline bool get_active_super(struct super_block *sb)
1bd87d01c81377 Christian Brauner 2025-03-29  1180  {
1bd87d01c81377 Christian Brauner 2025-03-29  1181  	bool active;
1bd87d01c81377 Christian Brauner 2025-03-29  1182  
1bd87d01c81377 Christian Brauner 2025-03-29  1183  	if (super_lock_excl(sb)) {
1bd87d01c81377 Christian Brauner 2025-03-29  1184  		active = atomic_inc_not_zero(&sb->s_active);
1bd87d01c81377 Christian Brauner 2025-03-29  1185  		super_unlock_excl(sb);
1bd87d01c81377 Christian Brauner 2025-03-29  1186  	}
1bd87d01c81377 Christian Brauner 2025-03-29 @1187  	return active;
1bd87d01c81377 Christian Brauner 2025-03-29  1188  }
1bd87d01c81377 Christian Brauner 2025-03-29  1189  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 0/6] Extend freeze support to suspend and hibernate

[Christian Brauner]

Add the necessary infrastructure changes to support freezing for suspend
and hibernate.

Just got back from LSFMM. So still jetlagged and likelihood of bugs
increased. This should all that's needed to wire up power.

This will be in vfs-6.16.super shortly.

---
Changes in v2:
- Don't grab reference in the iterator make that a requirement for the
  callers that need custom behavior.
- Link to v1: https://lore.kernel.org/r/20250328-work-freeze-v1-0-a2c3a6b0e7a6@kernel.org

---
Christian Brauner (6):
      super: remove pointless s_root checks
      super: simplify user_get_super()
      super: skip dying superblocks early
      super: use a common iterator (Part 1)
      super: use common iterator (Part 2)
      super: add filesystem freezing helpers for suspend and hibernate

 fs/super.c         | 201 ++++++++++++++++++++++++++++++++---------------------
 include/linux/fs.h |   4 +-
 2 files changed, 126 insertions(+), 79 deletions(-)
---
base-commit: acb4f33713b9f6cadb6143f211714c343465411c
change-id: 20250328-work-freeze-0a446869cd62

[PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[Christian Brauner]

Allow the power subsystem to support filesystem freeze for
suspend and hibernate.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/super.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |  2 ++
 2 files changed, 57 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 666a2a16df87..4364b763e91f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1176,6 +1176,61 @@ void emergency_thaw_all(void)
 	}
 }
 
+static inline bool get_active_super(struct super_block *sb)
+{
+	bool active;
+
+	if (super_lock_excl(sb)) {
+		active = atomic_inc_not_zero(&sb->s_active);
+		super_unlock_excl(sb);
+	}
+	return active;
+}
+
+static void filesystems_freeze_callback(struct super_block *sb, void *unused)
+{
+	if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super)
+		return;
+
+	if (!get_active_super(sb))
+		return;
+
+	if (sb->s_op->freeze_super)
+		sb->s_op->freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
+	else
+		freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
+
+	deactivate_super(sb);
+}
+
+void filesystems_freeze(bool hibernate)
+{
+	__iterate_supers(filesystems_freeze_callback, NULL,
+			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
+}
+
+static void filesystems_thaw_callback(struct super_block *sb, void *unused)
+{
+	if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super)
+		return;
+
+	if (!get_active_super(sb))
+		return;
+
+	if (sb->s_op->thaw_super)
+		sb->s_op->thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
+	else
+		thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
+
+	deactivate_super(sb);
+}
+
+void filesystems_thaw(bool hibernate)
+{
+	__iterate_supers(filesystems_thaw_callback, NULL,
+			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
+}
+
 static DEFINE_IDA(unnamed_dev_ida);
 
 /**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c475fa874055..29bd28491eff 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3518,6 +3518,8 @@ extern void drop_super_exclusive(struct super_block *sb);
 extern void iterate_supers(void (*f)(struct super_block *, void *), void *arg);
 extern void iterate_supers_type(struct file_system_type *,
 			        void (*)(struct super_block *, void *), void *);
+void filesystems_freeze(bool hibernate);
+void filesystems_thaw(bool hibernate);
 
 extern int dcache_dir_open(struct inode *, struct file *);
 extern int dcache_dir_close(struct inode *, struct file *);

-- 
2.47.2

Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[Christian Brauner]

On Sat, Mar 29, 2025 at 09:42:19AM +0100, Christian Brauner wrote:
> Allow the power subsystem to support filesystem freeze for
> suspend and hibernate.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/super.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h |  2 ++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 666a2a16df87..4364b763e91f 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1176,6 +1176,61 @@ void emergency_thaw_all(void)
>  	}
>  }
>  
> +static inline bool get_active_super(struct super_block *sb)
> +{
> +	bool active;

Typo on my end. This is ofc bool active = false;
And fixed.

> +
> +	if (super_lock_excl(sb)) {
> +		active = atomic_inc_not_zero(&sb->s_active);
> +		super_unlock_excl(sb);
> +	}
> +	return active;
> +}
> +
> +static void filesystems_freeze_callback(struct super_block *sb, void *unused)
> +{
> +	if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super)
> +		return;
> +
> +	if (!get_active_super(sb))
> +		return;
> +
> +	if (sb->s_op->freeze_super)
> +		sb->s_op->freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
> +	else
> +		freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
> +
> +	deactivate_super(sb);
> +}
> +
> +void filesystems_freeze(bool hibernate)
> +{
> +	__iterate_supers(filesystems_freeze_callback, NULL,
> +			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
> +}
> +
> +static void filesystems_thaw_callback(struct super_block *sb, void *unused)
> +{
> +	if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super)
> +		return;
> +
> +	if (!get_active_super(sb))
> +		return;
> +
> +	if (sb->s_op->thaw_super)
> +		sb->s_op->thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
> +	else
> +		thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL);
> +
> +	deactivate_super(sb);
> +}
> +
> +void filesystems_thaw(bool hibernate)
> +{
> +	__iterate_supers(filesystems_thaw_callback, NULL,
> +			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
> +}
> +
>  static DEFINE_IDA(unnamed_dev_ida);
>  
>  /**
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c475fa874055..29bd28491eff 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3518,6 +3518,8 @@ extern void drop_super_exclusive(struct super_block *sb);
>  extern void iterate_supers(void (*f)(struct super_block *, void *), void *arg);
>  extern void iterate_supers_type(struct file_system_type *,
>  			        void (*)(struct super_block *, void *), void *);
> +void filesystems_freeze(bool hibernate);
> +void filesystems_thaw(bool hibernate);
>  
>  extern int dcache_dir_open(struct inode *, struct file *);
>  extern int dcache_dir_close(struct inode *, struct file *);
> 
> -- 
> 2.47.2
> 

Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[kernel test robot]

Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on acb4f33713b9f6cadb6143f211714c343465411c]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Brauner/super-remove-pointless-s_root-checks/20250329-164716
base:   acb4f33713b9f6cadb6143f211714c343465411c
patch link:    https://lore.kernel.org/r/20250329-work-freeze-v2-6-a47af37ecc3d%40kernel.org
patch subject: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate
config: hexagon-randconfig-002-20250329 (https://download.01.org/0day-ci/archive/20250329/202503291904.PAJxnbuR-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 7eccafc3c84606587a175c0a8c1ebea6e4fb21cd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250329/202503291904.PAJxnbuR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503291904.PAJxnbuR-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/super.c:1183:6: warning: variable 'active' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    1183 |         if (super_lock_excl(sb)) {
         |             ^~~~~~~~~~~~~~~~~~~
   fs/super.c:1187:9: note: uninitialized use occurs here
    1187 |         return active;
         |                ^~~~~~
   fs/super.c:1183:2: note: remove the 'if' if its condition is always true
    1183 |         if (super_lock_excl(sb)) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   fs/super.c:1181:13: note: initialize the variable 'active' to silence this warning
    1181 |         bool active;
         |                    ^
         |                     = 0
   1 warning generated.


vim +1183 fs/super.c

  1178	
  1179	static inline bool get_active_super(struct super_block *sb)
  1180	{
  1181		bool active;
  1182	
> 1183		if (super_lock_excl(sb)) {
  1184			active = atomic_inc_not_zero(&sb->s_active);
  1185			super_unlock_excl(sb);
  1186		}
  1187		return active;
  1188	}
  1189	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[Jan Kara]

On Sat 29-03-25 09:42:19, Christian Brauner wrote:
> Allow the power subsystem to support filesystem freeze for
> suspend and hibernate.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

One comment below. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

> +void filesystems_thaw(bool hibernate)
> +{
> +	__iterate_supers(filesystems_thaw_callback, NULL,
> +			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
> +}

I think we should thaw in normal superblock order, not in reverse one? To
thaw the bottommost filesystem first? The filesystem thaw callback can
write to the underlying device and this could cause deadlocks...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 6/6] super: add filesystem freezing helpers for suspend and hibernate

[Christian Brauner]

On Mon, Mar 31, 2025 at 12:23:04PM +0200, Jan Kara wrote:
> On Sat 29-03-25 09:42:19, Christian Brauner wrote:
> > Allow the power subsystem to support filesystem freeze for
> > suspend and hibernate.
> > 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> One comment below. Otherwise feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> > +void filesystems_thaw(bool hibernate)
> > +{
> > +	__iterate_supers(filesystems_thaw_callback, NULL,
> > +			 SUPER_ITER_UNLOCKED | SUPER_ITER_REVERSE);
> > +}
> 
> I think we should thaw in normal superblock order, not in reverse one? To
> thaw the bottommost filesystem first? The filesystem thaw callback can
> write to the underlying device and this could cause deadlocks...

Yep, I've fixed that already up in vfs-6.16.super yesterday.
Sorry, forgot to mention that here. Thanks for noticing!