[PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Zhiwei Liang via B4 Relay]

From: Zhiwei Liang <zhiwei.liang@zliang.me>

When a suspend starts during FITRIM, fstrim can fail with:

  fstrim: /: FITRIM ioctl failed: Unknown error 512

On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
fstrim.service runs were in progress when an automatic suspend started.
All three failed this way and left the filesystem untrimmed.

Since commit 69313850dce3 ("btrfs: add cancellation points to trim
loops"), the trim loops return -ERESTARTSYS when
btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.

-ERESTARTSYS is only valid while a signal is pending, because only the
signal handling on syscall exit turns it into a restart. But
freeze_processes() first sets pm_freezing, which makes freezing() true
for every user task. Then it walks the task list and sends each task a
fake signal. The trim loops check btrfs_trim_interrupted() after every
extent. If a check falls in this window, the loop returns -ERESTARTSYS
with no signal pending. Nothing restarts the syscall. On that laptop,
with a few thousand threads, freezing user space took 2 to 5 ms in each
of the three suspends above, and all three runs hit this window.

Make btrfs_trim_interrupted() treat a freeze as an interruption only
once the fake signal has arrived, i.e. when
freezing(current) && signal_pending(current). freeze_processes() sends
that signal in the same call. The trim loop stops at the next check
after it, so suspend is not delayed.

The restarted syscall re-reads struct fstrim_range from user memory.
When the device phase is interrupted, btrfs_trim_fs() has already stored
the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
the struct back unconditionally. The restart is then asked to trim
[start, start + trimmed) instead of the requested range, or fails with
-EINVAL when nothing was counted yet. Skip the copy_to_user() for
-ERESTARTSYS. In all other cases the partial count is still copied back.

Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
during a trim leaked errno 512, and 10 of 10 restarts after a
device-phase interruption failed with -EINVAL. With the patch, all 30
runs restarted and trimmed the full amount.

Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
CC: stable@vger.kernel.org # 5.15+
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
---
 fs/btrfs/free-space-cache.h | 3 ++-
 fs/btrfs/ioctl.c            | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index e22443598b..cba2f5fd6b 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
 
 static inline bool btrfs_trim_interrupted(void)
 {
-	return fatal_signal_pending(current) || freezing(current);
+	return fatal_signal_pending(current) ||
+	       (freezing(current) && signal_pending(current));
 }
 
 /*
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ac070c745b..b02c59cbb0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
 	range.minlen = max(range.minlen, minlen);
 	ret = btrfs_trim_fs(fs_info, &range);
 
+	/*
+	 * A restart re-reads the range from user space, so do not clobber it
+	 * with the partial len the rerun would otherwise take as its input.
+	 */
+	if (ret == -ERESTARTSYS)
+		return ret;
+
 	if (copy_to_user(arg, &range, sizeof(range)))
 		return -EFAULT;
 

---
base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13

Best regards,
-- 
Zhiwei Liang <zhiwei.liang@zliang.me>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 17:27, Zhiwei Liang via B4 Relay 写道:
> From: Zhiwei Liang <zhiwei.liang@zliang.me>
> 
> When a suspend starts during FITRIM, fstrim can fail with:
> 
>    fstrim: /: FITRIM ioctl failed: Unknown error 512
> 
> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> fstrim.service runs were in progress when an automatic suspend started.
> All three failed this way and left the filesystem untrimmed.
> 
> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> loops"), the trim loops return -ERESTARTSYS when
> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
> 
> -ERESTARTSYS is only valid while a signal is pending, because only the
> signal handling on syscall exit turns it into a restart. But
> freeze_processes() first sets pm_freezing, which makes freezing() true
> for every user task. Then it walks the task list and sends each task a
> fake signal. The trim loops check btrfs_trim_interrupted() after every
> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> with no signal pending. Nothing restarts the syscall. On that laptop,
> with a few thousand threads, freezing user space took 2 to 5 ms in each
> of the three suspends above, and all three runs hit this window.
> 
> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> once the fake signal has arrived, i.e. when
> freezing(current) && signal_pending(current). freeze_processes() sends
> that signal in the same call. The trim loop stops at the next check
> after it, so suspend is not delayed.
> 
> The restarted syscall re-reads struct fstrim_range from user memory.
> When the device phase is interrupted, btrfs_trim_fs() has already stored
> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> the struct back unconditionally. The restart is then asked to trim
> [start, start + trimmed) instead of the requested range, or fails with
> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> -ERESTARTSYS. In all other cases the partial count is still copied back.
> 
> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> during a trim leaked errno 512, and 10 of 10 restarts after a
> device-phase interruption failed with -EINVAL. With the patch, all 30
> runs restarted and trimmed the full amount.
> 
> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> CC: stable@vger.kernel.org # 5.15+
> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> ---
>   fs/btrfs/free-space-cache.h | 3 ++-
>   fs/btrfs/ioctl.c            | 7 +++++++
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index e22443598b..cba2f5fd6b 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>   
>   static inline bool btrfs_trim_interrupted(void)
>   {
> -	return fatal_signal_pending(current) || freezing(current);
> +	return fatal_signal_pending(current) ||
> +	       (freezing(current) && signal_pending(current));

What if pm freeze has began but a real signal is also pending before the 
fake pm signal?

>   }
>   
>   /*
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ac070c745b..b02c59cbb0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>   	range.minlen = max(range.minlen, minlen);
>   	ret = btrfs_trim_fs(fs_info, &range);
>   
> +	/*
> +	 * A restart re-reads the range from user space, so do not clobber it
> +	 * with the partial len the rerun would otherwise take as its input.
> +	 */
> +	if (ret == -ERESTARTSYS)
> +		return ret;
> +
>   	if (copy_to_user(arg, &range, sizeof(range)))
>   		return -EFAULT;
>   
> 
> ---
> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
> 
> Best regards,

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
<devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
>
> From: Zhiwei Liang <zhiwei.liang@zliang.me>
>
> When a suspend starts during FITRIM, fstrim can fail with:
>
>   fstrim: /: FITRIM ioctl failed: Unknown error 512
>
> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> fstrim.service runs were in progress when an automatic suspend started.
> All three failed this way and left the filesystem untrimmed.
>
> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> loops"), the trim loops return -ERESTARTSYS when
> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
>
> -ERESTARTSYS is only valid while a signal is pending, because only the
> signal handling on syscall exit turns it into a restart. But
> freeze_processes() first sets pm_freezing, which makes freezing() true
> for every user task. Then it walks the task list and sends each task a
> fake signal. The trim loops check btrfs_trim_interrupted() after every
> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> with no signal pending. Nothing restarts the syscall. On that laptop,
> with a few thousand threads, freezing user space took 2 to 5 ms in each
> of the three suspends above, and all three runs hit this window.
>
> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> once the fake signal has arrived, i.e. when
> freezing(current) && signal_pending(current). freeze_processes() sends
> that signal in the same call. The trim loop stops at the next check
> after it, so suspend is not delayed.
Looking at how ext4 handles this scenario it seems to me they don't set
the error code to -ERESTARTSYS and (from a quick glance) return the
bytes discarded to the caller.
And like before they continue the discard (which will skip/fail again) until
there's nothing to be done.

I don't know what the intention of btrfs: handle user interrupt
properly in btrfs_trim_fs()
was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>
> The restarted syscall re-reads struct fstrim_range from user memory.
> When the device phase is interrupted, btrfs_trim_fs() has already stored
> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> the struct back unconditionally. The restart is then asked to trim
> [start, start + trimmed) instead of the requested range, or fails with
> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> -ERESTARTSYS. In all other cases the partial count is still copied back.
>
> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> during a trim leaked errno 512, and 10 of 10 restarts after a
> device-phase interruption failed with -EINVAL. With the patch, all 30
> runs restarted and trimmed the full amount.
>
> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> CC: stable@vger.kernel.org # 5.15+
> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> ---
>  fs/btrfs/free-space-cache.h | 3 ++-
>  fs/btrfs/ioctl.c            | 7 +++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index e22443598b..cba2f5fd6b 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>
>  static inline bool btrfs_trim_interrupted(void)
>  {
> -       return fatal_signal_pending(current) || freezing(current);
> +       return fatal_signal_pending(current) ||
> +              (freezing(current) && signal_pending(current));
>  }
>
>  /*
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ac070c745b..b02c59cbb0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>         range.minlen = max(range.minlen, minlen);
>         ret = btrfs_trim_fs(fs_info, &range);
>
> +       /*
> +        * A restart re-reads the range from user space, so do not clobber it
> +        * with the partial len the rerun would otherwise take as its input.
> +        */
> +       if (ret == -ERESTARTSYS)
> +               return ret;
> +
>         if (copy_to_user(arg, &range, sizeof(range)))
>                 return -EFAULT;
>
>
> ---
> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
>
> Best regards,
> --
> Zhiwei Liang <zhiwei.liang@zliang.me>
>
>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 19:21, Luca Stefani 写道:
> On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
> <devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
>>
>> From: Zhiwei Liang <zhiwei.liang@zliang.me>
>>
>> When a suspend starts during FITRIM, fstrim can fail with:
>>
>>    fstrim: /: FITRIM ioctl failed: Unknown error 512
>>
>> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
>> fstrim.service runs were in progress when an automatic suspend started.
>> All three failed this way and left the filesystem untrimmed.
>>
>> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
>> loops"), the trim loops return -ERESTARTSYS when
>> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
>> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
>> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
>>
>> -ERESTARTSYS is only valid while a signal is pending, because only the
>> signal handling on syscall exit turns it into a restart. But
>> freeze_processes() first sets pm_freezing, which makes freezing() true
>> for every user task. Then it walks the task list and sends each task a
>> fake signal. The trim loops check btrfs_trim_interrupted() after every
>> extent. If a check falls in this window, the loop returns -ERESTARTSYS
>> with no signal pending. Nothing restarts the syscall. On that laptop,
>> with a few thousand threads, freezing user space took 2 to 5 ms in each
>> of the three suspends above, and all three runs hit this window.
>>
>> Make btrfs_trim_interrupted() treat a freeze as an interruption only
>> once the fake signal has arrived, i.e. when
>> freezing(current) && signal_pending(current). freeze_processes() sends
>> that signal in the same call. The trim loop stops at the next check
>> after it, so suspend is not delayed.
> Looking at how ext4 handles this scenario it seems to me they don't set
> the error code to -ERESTARTSYS and (from a quick glance) return the
> bytes discarded to the caller.
> And like before they continue the discard (which will skip/fail again) until
> there's nothing to be done.
> 
> I don't know what the intention of btrfs: handle user interrupt
> properly in btrfs_trim_fs()
> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.

I agree on that btrfs is the exception, we should follow ext4/xfs, not 
rely on some tricky signal handling behavior for suspension.

I think the tokens of the author should be spent on unifying the btrfs 
fstrim suspension behavior instead.


>>
>> The restarted syscall re-reads struct fstrim_range from user memory.
>> When the device phase is interrupted, btrfs_trim_fs() has already stored
>> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
>> the struct back unconditionally. The restart is then asked to trim
>> [start, start + trimmed) instead of the requested range, or fails with
>> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
>> -ERESTARTSYS. In all other cases the partial count is still copied back.
>>
>> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
>> during a trim leaked errno 512, and 10 of 10 restarts after a
>> device-phase interruption failed with -EINVAL. With the patch, all 30
>> runs restarted and trimmed the full amount.
>>
>> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
>> CC: stable@vger.kernel.org # 5.15+
>> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
>> ---
>>   fs/btrfs/free-space-cache.h | 3 ++-
>>   fs/btrfs/ioctl.c            | 7 +++++++
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
>> index e22443598b..cba2f5fd6b 100644
>> --- a/fs/btrfs/free-space-cache.h
>> +++ b/fs/btrfs/free-space-cache.h
>> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>>
>>   static inline bool btrfs_trim_interrupted(void)
>>   {
>> -       return fatal_signal_pending(current) || freezing(current);
>> +       return fatal_signal_pending(current) ||
>> +              (freezing(current) && signal_pending(current));
>>   }
>>
>>   /*
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index ac070c745b..b02c59cbb0 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>>          range.minlen = max(range.minlen, minlen);
>>          ret = btrfs_trim_fs(fs_info, &range);
>>
>> +       /*
>> +        * A restart re-reads the range from user space, so do not clobber it
>> +        * with the partial len the rerun would otherwise take as its input.
>> +        */
>> +       if (ret == -ERESTARTSYS)
>> +               return ret;
>> +
>>          if (copy_to_user(arg, &range, sizeof(range)))
>>                  return -EFAULT;
>>
>>
>> ---
>> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
>> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
>>
>> Best regards,
>> --
>> Zhiwei Liang <zhiwei.liang@zliang.me>
>>
>>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2026/10/28 19:21, Luca Stefani 写道:
> > On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
> > <devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
> >>
> >> From: Zhiwei Liang <zhiwei.liang@zliang.me>
> >>
> >> When a suspend starts during FITRIM, fstrim can fail with:
> >>
> >>    fstrim: /: FITRIM ioctl failed: Unknown error 512
> >>
> >> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> >> fstrim.service runs were in progress when an automatic suspend started.
> >> All three failed this way and left the filesystem untrimmed.
> >>
> >> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> >> loops"), the trim loops return -ERESTARTSYS when
> >> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> >> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> >> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
> >>
> >> -ERESTARTSYS is only valid while a signal is pending, because only the
> >> signal handling on syscall exit turns it into a restart. But
> >> freeze_processes() first sets pm_freezing, which makes freezing() true
> >> for every user task. Then it walks the task list and sends each task a
> >> fake signal. The trim loops check btrfs_trim_interrupted() after every
> >> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> >> with no signal pending. Nothing restarts the syscall. On that laptop,
> >> with a few thousand threads, freezing user space took 2 to 5 ms in each
> >> of the three suspends above, and all three runs hit this window.
> >>
> >> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> >> once the fake signal has arrived, i.e. when
> >> freezing(current) && signal_pending(current). freeze_processes() sends
> >> that signal in the same call. The trim loop stops at the next check
> >> after it, so suspend is not delayed.
> > Looking at how ext4 handles this scenario it seems to me they don't set
> > the error code to -ERESTARTSYS and (from a quick glance) return the
> > bytes discarded to the caller.
> > And like before they continue the discard (which will skip/fail again) until
> > there's nothing to be done.
> >
> > I don't know what the intention of btrfs: handle user interrupt
> > properly in btrfs_trim_fs()
> > was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>
> I agree on that btrfs is the exception, we should follow ext4/xfs, not
> rely on some tricky signal handling behavior for suspension.
When I updated btrfs_trim_interrupted I used the same "checks" as
seen in ext4 so at least on that part it's identical.

What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
ext4_try_to_trim_range only ever succeeds by returning the count of
trimmed bytes
even if it checks for trim interruptions.

I think if we remove ret = -ERESTARTSYS; from the
btrfs_trim_interrupted branches
we'd align the behavior (plus reverting the previous change as that's
now useless)
>
> I think the tokens of the author should be spent on unifying the btrfs
> fstrim suspension behavior instead.
>
>
> >>
> >> The restarted syscall re-reads struct fstrim_range from user memory.
> >> When the device phase is interrupted, btrfs_trim_fs() has already stored
> >> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> >> the struct back unconditionally. The restart is then asked to trim
> >> [start, start + trimmed) instead of the requested range, or fails with
> >> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> >> -ERESTARTSYS. In all other cases the partial count is still copied back.
> >>
> >> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> >> during a trim leaked errno 512, and 10 of 10 restarts after a
> >> device-phase interruption failed with -EINVAL. With the patch, all 30
> >> runs restarted and trimmed the full amount.
> >>
> >> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> >> CC: stable@vger.kernel.org # 5.15+
> >> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> >> ---
> >>   fs/btrfs/free-space-cache.h | 3 ++-
> >>   fs/btrfs/ioctl.c            | 7 +++++++
> >>   2 files changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> >> index e22443598b..cba2f5fd6b 100644
> >> --- a/fs/btrfs/free-space-cache.h
> >> +++ b/fs/btrfs/free-space-cache.h
> >> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
> >>
> >>   static inline bool btrfs_trim_interrupted(void)
> >>   {
> >> -       return fatal_signal_pending(current) || freezing(current);
> >> +       return fatal_signal_pending(current) ||
> >> +              (freezing(current) && signal_pending(current));
> >>   }
> >>
> >>   /*
> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> >> index ac070c745b..b02c59cbb0 100644
> >> --- a/fs/btrfs/ioctl.c
> >> +++ b/fs/btrfs/ioctl.c
> >> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
> >>          range.minlen = max(range.minlen, minlen);
> >>          ret = btrfs_trim_fs(fs_info, &range);
> >>
> >> +       /*
> >> +        * A restart re-reads the range from user space, so do not clobber it
> >> +        * with the partial len the rerun would otherwise take as its input.
> >> +        */
> >> +       if (ret == -ERESTARTSYS)
> >> +               return ret;
> >> +
> >>          if (copy_to_user(arg, &range, sizeof(range)))
> >>                  return -EFAULT;
> >>
> >>
> >> ---
> >> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> >> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
> >>
> >> Best regards,
> >> --
> >> Zhiwei Liang <zhiwei.liang@zliang.me>
> >>
> >>
>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 19:50, Luca Stefani 写道:
> On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
[...]
>>> I don't know what the intention of btrfs: handle user interrupt
>>> properly in btrfs_trim_fs()
>>> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>>
>> I agree on that btrfs is the exception, we should follow ext4/xfs, not
>> rely on some tricky signal handling behavior for suspension.
> When I updated btrfs_trim_interrupted I used the same "checks" as
> seen in ext4 so at least on that part it's identical.
> 
> What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
> ext4_try_to_trim_range only ever succeeds by returning the count of
> trimmed bytes
> even if it checks for trim interruptions.
> 
> I think if we remove ret = -ERESTARTSYS; from the
> btrfs_trim_interrupted branches
> we'd align the behavior (plus reverting the previous change as that's
> now useless)

I think that's would work for most call sites, but there are some call 
sites that we may want to know if it's interrupted.

E.g. btrfs_issue_discard() is very deep inside all trim calls, if we 
drop the return value without any other extra way to notify the caller, 
it will incorrectly return 0, which is definitely not a good idea.

A safer solution would be keep the existing ERESTARTSYS, but change it 
to return 0 for now.

But if you have a better solution and can completely remove ERESTARTSYS, 
I can't be more happier.

Thanks,
Qu

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 20:12, Qu Wenruo 写道:
> 
> 
> 在 2026/10/28 19:50, Luca Stefani 写道:
>> On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
> [...]
>>>> I don't know what the intention of btrfs: handle user interrupt
>>>> properly in btrfs_trim_fs()
>>>> was, maybe we should just swallow the ERESTARTSYS case in those 
>>>> scenarios.
>>>
>>> I agree on that btrfs is the exception, we should follow ext4/xfs, not
>>> rely on some tricky signal handling behavior for suspension.
>> When I updated btrfs_trim_interrupted I used the same "checks" as
>> seen in ext4 so at least on that part it's identical.
>>
>> What differs is our error handling: we set/handle -ERESTARTSYS, ext4 
>> does not.
>> ext4_try_to_trim_range only ever succeeds by returning the count of
>> trimmed bytes
>> even if it checks for trim interruptions.
>>
>> I think if we remove ret = -ERESTARTSYS; from the
>> btrfs_trim_interrupted branches
>> we'd align the behavior (plus reverting the previous change as that's
>> now useless)
> 
> I think that's would work for most call sites, but there are some call 
> sites that we may want to know if it's interrupted.
> 
> E.g. btrfs_issue_discard() is very deep inside all trim calls, if we 
> drop the return value without any other extra way to notify the caller, 
> it will incorrectly return 0, which is definitely not a good idea.
> 
> A safer solution would be keep the existing ERESTARTSYS, but change it 
> to return 0 for now.

Sorry, I mean to return 0 at the fstrim entrance, btrfs_trim_fs().

> 
> But if you have a better solution and can completely remove ERESTARTSYS, 
> I can't be more happier.
> 
> Thanks,
> Qu

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 10:42, Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2026/10/28 19:50, Luca Stefani 写道:
> > On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
> [...]
> >>> I don't know what the intention of btrfs: handle user interrupt
> >>> properly in btrfs_trim_fs()
> >>> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
> >>
> >> I agree on that btrfs is the exception, we should follow ext4/xfs, not
> >> rely on some tricky signal handling behavior for suspension.
> > When I updated btrfs_trim_interrupted I used the same "checks" as
> > seen in ext4 so at least on that part it's identical.
> >
> > What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
> > ext4_try_to_trim_range only ever succeeds by returning the count of
> > trimmed bytes
> > even if it checks for trim interruptions.
> >
> > I think if we remove ret = -ERESTARTSYS; from the
> > btrfs_trim_interrupted branches
> > we'd align the behavior (plus reverting the previous change as that's
> > now useless)
>
> I think that's would work for most call sites, but there are some call
> sites that we may want to know if it's interrupted.
>
> E.g. btrfs_issue_discard() is very deep inside all trim calls, if we
> drop the return value without any other extra way to notify the caller,
> it will incorrectly return 0, which is definitely not a good idea.
>
> A safer solution would be keep the existing ERESTARTSYS, but change it
> to return 0 for now.
That would work, I think.
But that would only solve the userspace problem, as checking for
freezing(current) might be wrong in those points, which was pointed
out in this change.
Maybe the "old" btrfs_trim_interrupted users should go back to
fatal_signal_pending(current) only while the free discarding phase
keep it.

This looks similar to what some places in ext4 do, but I'm no expert.
>
> But if you have a better solution and can completely remove ERESTARTSYS,
> I can't be more happier.
I wish I could help, but as long as it's a targeted change I'd be
happy to help, but I don't know enough about the fs :)
>
> Thanks,
> Qu