| Message ID | 20230921013945.559634-4-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: reduce AGF hold times during fstrim operations | expand |
On Thu, Sep 21, 2023 at 11:39:45AM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > A recent ext4 patch posting from Jan Kara reminded me of a > discussion a year ago about fstrim in progress preventing kernels > from suspending. The fix is simple, we should do the same for XFS. > > This removes the -ERESTARTSYS error return from this code, replacing > it with either the last error seen or the number of blocks > successfully trimmed up to the point where we detected the stop > condition. Kinda weird, actually, that FITRIM can do some discard work, hit an error, and return the error without updating userspace about the work that it /did/ get done. Short writes work like that, so why not this? Oh. This is one of those syscalls that has no manpage, nor a description in Documentation/, so the behavior of this is entirely defined by calling code interpretation. Yeah, not doing that today. > References: https://bugzilla.kernel.org/show_bug.cgi?id=216322 > Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/xfs_discard.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > index f16b254b5eaa..d5787991bb5b 100644 > --- a/fs/xfs/xfs_discard.c > +++ b/fs/xfs/xfs_discard.c > @@ -283,6 +283,12 @@ xfs_trim_gather_extents( > return error; > } > > +static bool > +xfs_trim_should_stop(void) > +{ > + return fatal_signal_pending(current) || freezing(current); > +} > + > /* > * Iterate the free list gathering extents and discarding them. We need a cursor > * for the repeated iteration of gather/discard loop, so use the longest extent > @@ -336,10 +342,9 @@ xfs_trim_extents( > if (error) > break; > > - if (fatal_signal_pending(current)) { > - error = -ERESTARTSYS; > + if (xfs_trim_should_stop()) > break; > - } > + > } while (tcur.ar_blockcount != 0); > > return error; > @@ -408,12 +413,12 @@ xfs_ioc_trim( > for_each_perag_range(mp, agno, xfs_daddr_to_agno(mp, end), pag) { > error = xfs_trim_extents(pag, start, end, minlen, > &blocks_trimmed); > - if (error) { > + if (error) > last_error = error; > - if (error == -ERESTARTSYS) { > - xfs_perag_rele(pag); > - break; > - } > + > + if (xfs_trim_should_stop()) { > + xfs_perag_rele(pag); > + break; > } > } > > -- > 2.40.1 >
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index f16b254b5eaa..d5787991bb5b 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -283,6 +283,12 @@ xfs_trim_gather_extents( return error; } +static bool +xfs_trim_should_stop(void) +{ + return fatal_signal_pending(current) || freezing(current); +} + /* * Iterate the free list gathering extents and discarding them. We need a cursor * for the repeated iteration of gather/discard loop, so use the longest extent @@ -336,10 +342,9 @@ xfs_trim_extents( if (error) break; - if (fatal_signal_pending(current)) { - error = -ERESTARTSYS; + if (xfs_trim_should_stop()) break; - } + } while (tcur.ar_blockcount != 0); return error; @@ -408,12 +413,12 @@ xfs_ioc_trim( for_each_perag_range(mp, agno, xfs_daddr_to_agno(mp, end), pag) { error = xfs_trim_extents(pag, start, end, minlen, &blocks_trimmed); - if (error) { + if (error) last_error = error; - if (error == -ERESTARTSYS) { - xfs_perag_rele(pag); - break; - } + + if (xfs_trim_should_stop()) { + xfs_perag_rele(pag); + break; } }