[PATCH 6.6.x] btrfs: add cancellation points to trim loops

[PATCH 6.6.x] btrfs: add cancellation points to trim loops

[David Sterba]

From: Luca Stefani <luca.stefani.ge1@gmail.com>

There are reports that system cannot suspend due to running trim because
the task responsible for trimming the device isn't able to finish in
time, especially since we have a free extent discarding phase, which can
trim a lot of unallocated space. There are no limits on the trim size
(unlike the block group part).

Since trime isn't a critical call it can be interrupted at any time,
in such cases we stop the trim, report the amount of discarded bytes and
return an error.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
CC: stable@vger.kernel.org # 5.15+
Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c      | 7 ++++++-
 fs/btrfs/free-space-cache.c | 4 ++--
 fs/btrfs/free-space-cache.h | 7 +++++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index b3680e1c7054..599407120513 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1319,6 +1319,11 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
 		start += bytes_to_discard;
 		bytes_left -= bytes_to_discard;
 		*discarded_bytes += bytes_to_discard;
+
+		if (btrfs_trim_interrupted()) {
+			ret = -ERESTARTSYS;
+			break;
+		}
 	}
 
 	return ret;
@@ -6094,7 +6099,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
 		start += len;
 		*trimmed += bytes;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			ret = -ERESTARTSYS;
 			break;
 		}
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 3bcf4a30cad7..9a6ec9344c3e 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -3808,7 +3808,7 @@ static int trim_no_bitmap(struct btrfs_block_group *block_group,
 		if (async && *total_trimmed)
 			break;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			ret = -ERESTARTSYS;
 			break;
 		}
@@ -3999,7 +3999,7 @@ static int trim_bitmaps(struct btrfs_block_group *block_group,
 		}
 		block_group->discard_cursor = start;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			if (start != offset)
 				reset_trimming_bitmap(ctl, offset);
 			ret = -ERESTARTSYS;
diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index 33b4da3271b1..bd80c7b2af96 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -6,6 +6,8 @@
 #ifndef BTRFS_FREE_SPACE_CACHE_H
 #define BTRFS_FREE_SPACE_CACHE_H
 
+#include <linux/freezer.h>
+
 /*
  * This is the trim state of an extent or bitmap.
  *
@@ -43,6 +45,11 @@ static inline bool btrfs_free_space_trimming_bitmap(
 	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
 }
 
+static inline bool btrfs_trim_interrupted(void)
+{
+	return fatal_signal_pending(current) || freezing(current);
+}
+
 /*
  * Deltas are an effective way to populate global statistics.  Give macro names
  * to make it clear what we're doing.  An example is discard_extents in
-- 
2.45.0

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-3

Rule: The upstream commit ID must be specified with a separate line above the commit text.
Subject: [PATCH 6.6.x] btrfs: add cancellation points to trim loops
Link: https://lore.kernel.org/stable/20241125180729.13148-1-dsterba%40suse.com

Please ignore this mail if the patch is not relevant for upstream.

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

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

Found matching upstream commit: 69313850dce33ce8c24b38576a279421f4c60996

WARNING: Author mismatch between patch and found commit:
Backport author: David Sterba <dsterba@suse.com>
Commit author: Luca Stefani <luca.stefani.ge1@gmail.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.11.y | Present (different SHA1: fba6544ff4bf)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
--- -	2024-11-25 13:53:18.073145886 -0500
+++ /tmp/tmp.ws7hjJgxh2	2024-11-25 13:53:18.068806643 -0500
@@ -17,14 +17,14 @@
 ---
  fs/btrfs/extent-tree.c      | 7 ++++++-
  fs/btrfs/free-space-cache.c | 4 ++--
- fs/btrfs/free-space-cache.h | 6 ++++++
- 3 files changed, 14 insertions(+), 3 deletions(-)
+ fs/btrfs/free-space-cache.h | 7 +++++++
+ 3 files changed, 15 insertions(+), 3 deletions(-)
 
 diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
-index ad70548d1f722..d9f511babd89a 100644
+index b3680e1c7054..599407120513 100644
 --- a/fs/btrfs/extent-tree.c
 +++ b/fs/btrfs/extent-tree.c
-@@ -1316,6 +1316,11 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
+@@ -1319,6 +1319,11 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
  		start += bytes_to_discard;
  		bytes_left -= bytes_to_discard;
  		*discarded_bytes += bytes_to_discard;
@@ -36,7 +36,7 @@
  	}
  
  	return ret;
-@@ -6470,7 +6475,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
+@@ -6094,7 +6099,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
  		start += len;
  		*trimmed += bytes;
  
@@ -46,10 +46,10 @@
  			break;
  		}
 diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
-index eaa1dbd313528..f4bcb25306606 100644
+index 3bcf4a30cad7..9a6ec9344c3e 100644
 --- a/fs/btrfs/free-space-cache.c
 +++ b/fs/btrfs/free-space-cache.c
-@@ -3809,7 +3809,7 @@ static int trim_no_bitmap(struct btrfs_block_group *block_group,
+@@ -3808,7 +3808,7 @@ static int trim_no_bitmap(struct btrfs_block_group *block_group,
  		if (async && *total_trimmed)
  			break;
  
@@ -58,7 +58,7 @@
  			ret = -ERESTARTSYS;
  			break;
  		}
-@@ -4000,7 +4000,7 @@ static int trim_bitmaps(struct btrfs_block_group *block_group,
+@@ -3999,7 +3999,7 @@ static int trim_bitmaps(struct btrfs_block_group *block_group,
  		}
  		block_group->discard_cursor = start;
  
@@ -68,18 +68,19 @@
  				reset_trimming_bitmap(ctl, offset);
  			ret = -ERESTARTSYS;
 diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
-index 83774bfd7b3bb..9f1dbfdee8cab 100644
+index 33b4da3271b1..bd80c7b2af96 100644
 --- a/fs/btrfs/free-space-cache.h
 +++ b/fs/btrfs/free-space-cache.h
-@@ -10,6 +10,7 @@
- #include <linux/list.h>
- #include <linux/spinlock.h>
- #include <linux/mutex.h>
-+#include <linux/freezer.h>
- #include "fs.h"
+@@ -6,6 +6,8 @@
+ #ifndef BTRFS_FREE_SPACE_CACHE_H
+ #define BTRFS_FREE_SPACE_CACHE_H
  
- struct inode;
-@@ -56,6 +57,11 @@ static inline bool btrfs_free_space_trimming_bitmap(
++#include <linux/freezer.h>
++
+ /*
+  * This is the trim state of an extent or bitmap.
+  *
+@@ -43,6 +45,11 @@ static inline bool btrfs_free_space_trimming_bitmap(
  	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
  }
  
@@ -91,3 +92,6 @@
  /*
   * Deltas are an effective way to populate global statistics.  Give macro names
   * to make it clear what we're doing.  An example is discard_extents in
+-- 
+2.45.0
+
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[Greg KH]

On Mon, Nov 25, 2024 at 07:07:28PM +0100, David Sterba wrote:
> From: Luca Stefani <luca.stefani.ge1@gmail.com>
> 
> There are reports that system cannot suspend due to running trim because
> the task responsible for trimming the device isn't able to finish in
> time, especially since we have a free extent discarding phase, which can
> trim a lot of unallocated space. There are no limits on the trim size
> (unlike the block group part).
> 
> Since trime isn't a critical call it can be interrupted at any time,
> in such cases we stop the trim, report the amount of discarded bytes and
> return an error.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
> CC: stable@vger.kernel.org # 5.15+
> Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---

No git id?  :(

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[David Sterba]

On Mon, Dec 02, 2024 at 12:15:54PM +0100, Greg KH wrote:
> On Mon, Nov 25, 2024 at 07:07:28PM +0100, David Sterba wrote:
> > From: Luca Stefani <luca.stefani.ge1@gmail.com>
> > 
> > There are reports that system cannot suspend due to running trim because
> > the task responsible for trimming the device isn't able to finish in
> > time, especially since we have a free extent discarding phase, which can
> > trim a lot of unallocated space. There are no limits on the trim size
> > (unlike the block group part).
> > 
> > Since trime isn't a critical call it can be interrupted at any time,
> > in such cases we stop the trim, report the amount of discarded bytes and
> > return an error.
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
> > Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
> > CC: stable@vger.kernel.org # 5.15+
> > Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
> > Reviewed-by: David Sterba <dsterba@suse.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> 
> No git id?  :(

I forgot to add it but meanwhile Sasha looked up the commit and added it for me.

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[David Sterba]

On Tue, Dec 03, 2024 at 04:27:56PM +0100, David Sterba wrote:
> On Mon, Dec 02, 2024 at 12:15:54PM +0100, Greg KH wrote:
> > On Mon, Nov 25, 2024 at 07:07:28PM +0100, David Sterba wrote:
> > > From: Luca Stefani <luca.stefani.ge1@gmail.com>
> > > 
> > > There are reports that system cannot suspend due to running trim because
> > > the task responsible for trimming the device isn't able to finish in
> > > time, especially since we have a free extent discarding phase, which can
> > > trim a lot of unallocated space. There are no limits on the trim size
> > > (unlike the block group part).
> > > 
> > > Since trime isn't a critical call it can be interrupted at any time,
> > > in such cases we stop the trim, report the amount of discarded bytes and
> > > return an error.
> > > 
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
> > > Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
> > > CC: stable@vger.kernel.org # 5.15+
> > > Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
> > > Reviewed-by: David Sterba <dsterba@suse.com>
> > > Signed-off-by: David Sterba <dsterba@suse.com>
> > > ---
> > 
> > No git id?  :(
> 
> I forgot to add it but meanwhile Sasha looked up the commit and added it for me.

For the record it's 69313850dce33ce8c24b38576a279421f4c60996

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[Greg KH]

On Tue, Dec 03, 2024 at 04:41:59PM +0100, David Sterba wrote:
> On Tue, Dec 03, 2024 at 04:27:56PM +0100, David Sterba wrote:
> > On Mon, Dec 02, 2024 at 12:15:54PM +0100, Greg KH wrote:
> > > On Mon, Nov 25, 2024 at 07:07:28PM +0100, David Sterba wrote:
> > > > From: Luca Stefani <luca.stefani.ge1@gmail.com>
> > > > 
> > > > There are reports that system cannot suspend due to running trim because
> > > > the task responsible for trimming the device isn't able to finish in
> > > > time, especially since we have a free extent discarding phase, which can
> > > > trim a lot of unallocated space. There are no limits on the trim size
> > > > (unlike the block group part).
> > > > 
> > > > Since trime isn't a critical call it can be interrupted at any time,
> > > > in such cases we stop the trim, report the amount of discarded bytes and
> > > > return an error.
> > > > 
> > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
> > > > Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
> > > > CC: stable@vger.kernel.org # 5.15+
> > > > Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
> > > > Reviewed-by: David Sterba <dsterba@suse.com>
> > > > Signed-off-by: David Sterba <dsterba@suse.com>
> > > > ---
> > > 
> > > No git id?  :(
> > 
> > I forgot to add it but meanwhile Sasha looked up the commit and added it for me.
> 
> For the record it's 69313850dce33ce8c24b38576a279421f4c60996

Great, can you resend it with that information?

thanks,

greg k-h

[PATCH 6.6.x] btrfs: add cancellation points to trim loops

[David Sterba]

From: Luca Stefani <luca.stefani.ge1@gmail.com>

commit 69313850dce33ce8c24b38576a279421f4c60996 upstream.

There are reports that system cannot suspend due to running trim because
the task responsible for trimming the device isn't able to finish in
time, especially since we have a free extent discarding phase, which can
trim a lot of unallocated space. There are no limits on the trim size
(unlike the block group part).

Since trime isn't a critical call it can be interrupted at any time,
in such cases we stop the trim, report the amount of discarded bytes and
return an error.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=219180
Link: https://bugzilla.suse.com/show_bug.cgi?id=1229737
CC: stable@vger.kernel.org # 5.15+
Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c      | 7 ++++++-
 fs/btrfs/free-space-cache.c | 4 ++--
 fs/btrfs/free-space-cache.h | 7 +++++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index b3680e1c7054..599407120513 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1319,6 +1319,11 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
 		start += bytes_to_discard;
 		bytes_left -= bytes_to_discard;
 		*discarded_bytes += bytes_to_discard;
+
+		if (btrfs_trim_interrupted()) {
+			ret = -ERESTARTSYS;
+			break;
+		}
 	}
 
 	return ret;
@@ -6094,7 +6099,7 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
 		start += len;
 		*trimmed += bytes;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			ret = -ERESTARTSYS;
 			break;
 		}
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 3bcf4a30cad7..9a6ec9344c3e 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -3808,7 +3808,7 @@ static int trim_no_bitmap(struct btrfs_block_group *block_group,
 		if (async && *total_trimmed)
 			break;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			ret = -ERESTARTSYS;
 			break;
 		}
@@ -3999,7 +3999,7 @@ static int trim_bitmaps(struct btrfs_block_group *block_group,
 		}
 		block_group->discard_cursor = start;
 
-		if (fatal_signal_pending(current)) {
+		if (btrfs_trim_interrupted()) {
 			if (start != offset)
 				reset_trimming_bitmap(ctl, offset);
 			ret = -ERESTARTSYS;
diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index 33b4da3271b1..bd80c7b2af96 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -6,6 +6,8 @@
 #ifndef BTRFS_FREE_SPACE_CACHE_H
 #define BTRFS_FREE_SPACE_CACHE_H
 
+#include <linux/freezer.h>
+
 /*
  * This is the trim state of an extent or bitmap.
  *
@@ -43,6 +45,11 @@ static inline bool btrfs_free_space_trimming_bitmap(
 	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
 }
 
+static inline bool btrfs_trim_interrupted(void)
+{
+	return fatal_signal_pending(current) || freezing(current);
+}
+
 /*
  * Deltas are an effective way to populate global statistics.  Give macro names
  * to make it clear what we're doing.  An example is discard_extents in
-- 
2.45.0

Re: [PATCH 6.6.x] btrfs: add cancellation points to trim loops

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 69313850dce33ce8c24b38576a279421f4c60996

WARNING: Author mismatch between patch and upstream commit:
Backport author: David Sterba <dsterba@suse.com>
Commit author: Luca Stefani <luca.stefani.ge1@gmail.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  69313850dce33 ! 1:  949c2e944f056 btrfs: add cancellation points to trim loops
    @@ Metadata
      ## Commit message ##
         btrfs: add cancellation points to trim loops
     
    +    commit 69313850dce33ce8c24b38576a279421f4c60996 upstream.
    +
         There are reports that system cannot suspend due to running trim because
         the task responsible for trimming the device isn't able to finish in
         time, especially since we have a free extent discarding phase, which can
    @@ fs/btrfs/free-space-cache.c: static int trim_bitmaps(struct btrfs_block_group *b
     
      ## fs/btrfs/free-space-cache.h ##
     @@
    - #include <linux/list.h>
    - #include <linux/spinlock.h>
    - #include <linux/mutex.h>
    -+#include <linux/freezer.h>
    - #include "fs.h"
    + #ifndef BTRFS_FREE_SPACE_CACHE_H
    + #define BTRFS_FREE_SPACE_CACHE_H
      
    - struct inode;
    ++#include <linux/freezer.h>
    ++
    + /*
    +  * This is the trim state of an extent or bitmap.
    +  *
     @@ fs/btrfs/free-space-cache.h: static inline bool btrfs_free_space_trimming_bitmap(
      	return (info->trim_state == BTRFS_TRIM_STATE_TRIMMING);
      }
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |