Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace
From: Luca Stefani
Date: Wed Oct 28 2026 - 06:24:04 EST
On Wed, 28 Oct 2026 at 10:42, Qu Wenruo <wqu@xxxxxxxx> wrote:
>
>
>
> 在 2026/10/28 19:50, Luca Stefani 写道:
> > On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@xxxxxxxx> 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