| Message ID | 1493282368-32422-1-git-send-email-lczerner@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 4/27/17 3:39 AM, Lukas Czerner wrote: > fstrim can take really long time on big, slow device or on file system > with a lots of allocation groups. Currently there is no way for the user > to cancell the operation. This patch makes it possible for the user to > kill fstrim pocess by adding the check for fatal_signal_pending() in > xfs_trim_extents(). With this patch, it seems that if it is killed, then nothing is copied back to the user about the number of blocks trimmed before the kill. Is that intentional? I think it could be done in a way that the progress until the kill does get reported, and that might be more useful? Thanks, -Eric > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> > --- > fs/xfs/xfs_discard.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > index d796ffa..6a05d27 100644 > --- a/fs/xfs/xfs_discard.c > +++ b/fs/xfs/xfs_discard.c > @@ -132,6 +132,11 @@ xfs_trim_extents( > error = xfs_btree_decrement(cur, 0, &i); > if (error) > goto out_del_cursor; > + > + if (fatal_signal_pending(current)) { > + error = -ERESTARTSYS; > + goto out_del_cursor; > + } > } > > out_del_cursor: > @@ -196,8 +201,11 @@ xfs_ioc_trim( > for (agno = start_agno; agno <= end_agno; agno++) { > error = xfs_trim_extents(mp, agno, start, end, minlen, > &blocks_trimmed); > - if (error) > + if (error) { > last_error = error; > + if (error == -ERESTARTSYS) > + break; > + } > } > > if (last_error) > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 27, 2017 at 09:34:04AM -0500, Eric Sandeen wrote: > On 4/27/17 3:39 AM, Lukas Czerner wrote: > > fstrim can take really long time on big, slow device or on file system > > with a lots of allocation groups. Currently there is no way for the user > > to cancell the operation. This patch makes it possible for the user to > > kill fstrim pocess by adding the check for fatal_signal_pending() in > > xfs_trim_extents(). > > With this patch, it seems that if it is killed, then nothing is copied > back to the user about the number of blocks trimmed before the kill. > > Is that intentional? > > I think it could be done in a way that the progress until the kill > does get reported, and that might be more useful? AFAICT ext4 doesn't report anything if somoene ERESTARTSYS's it... > > Thanks, > -Eric > > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> > > --- > > fs/xfs/xfs_discard.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > > index d796ffa..6a05d27 100644 > > --- a/fs/xfs/xfs_discard.c > > +++ b/fs/xfs/xfs_discard.c > > @@ -132,6 +132,11 @@ xfs_trim_extents( > > error = xfs_btree_decrement(cur, 0, &i); > > if (error) > > goto out_del_cursor; > > + > > + if (fatal_signal_pending(current)) { ...but the process isn't going to be around to care anyway, right? --D > > + error = -ERESTARTSYS; > > + goto out_del_cursor; > > + } > > } > > > > out_del_cursor: > > @@ -196,8 +201,11 @@ xfs_ioc_trim( > > for (agno = start_agno; agno <= end_agno; agno++) { > > error = xfs_trim_extents(mp, agno, start, end, minlen, > > &blocks_trimmed); > > - if (error) > > + if (error) { > > last_error = error; > > + if (error == -ERESTARTSYS) > > + break; > > + } > > } > > > > if (last_error) > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/27/17 10:38 AM, Darrick J. Wong wrote: > On Thu, Apr 27, 2017 at 09:34:04AM -0500, Eric Sandeen wrote: >> On 4/27/17 3:39 AM, Lukas Czerner wrote: >>> fstrim can take really long time on big, slow device or on file system >>> with a lots of allocation groups. Currently there is no way for the user >>> to cancell the operation. This patch makes it possible for the user to >>> kill fstrim pocess by adding the check for fatal_signal_pending() in >>> xfs_trim_extents(). >> >> With this patch, it seems that if it is killed, then nothing is copied >> back to the user about the number of blocks trimmed before the kill. >> >> Is that intentional? >> >> I think it could be done in a way that the progress until the kill >> does get reported, and that might be more useful? > > AFAICT ext4 doesn't report anything if somoene ERESTARTSYS's it... Well we aren't ext4, are we? ;) actually, it seems that it may report "count" as ERESTARTSYS in: ext4_debug("trimmed %d blocks in the group %d\n", count, group); But yeah, although it does update the &range values, they don't get copied out on any error. Anyway, ok, ext4 doesn't. Is there a reason that we shouldn't? Thanks, -Eric >> >> Thanks, >> -Eric >> >>> Signed-off-by: Lukas Czerner <lczerner@redhat.com> >>> Reported-by: Zdenek Kabelac <zkabelac@redhat.com> >>> --- >>> fs/xfs/xfs_discard.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c >>> index d796ffa..6a05d27 100644 >>> --- a/fs/xfs/xfs_discard.c >>> +++ b/fs/xfs/xfs_discard.c >>> @@ -132,6 +132,11 @@ xfs_trim_extents( >>> error = xfs_btree_decrement(cur, 0, &i); >>> if (error) >>> goto out_del_cursor; >>> + >>> + if (fatal_signal_pending(current)) { > > ...but the process isn't going to be around to care anyway, right? > > --D > >>> + error = -ERESTARTSYS; >>> + goto out_del_cursor; >>> + } >>> } >>> >>> out_del_cursor: >>> @@ -196,8 +201,11 @@ xfs_ioc_trim( >>> for (agno = start_agno; agno <= end_agno; agno++) { >>> error = xfs_trim_extents(mp, agno, start, end, minlen, >>> &blocks_trimmed); >>> - if (error) >>> + if (error) { >>> last_error = error; >>> + if (error == -ERESTARTSYS) >>> + break; >>> + } >>> } >>> >>> if (last_error) >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/27/17 10:46 AM, Eric Sandeen wrote: > On 4/27/17 10:38 AM, Darrick J. Wong wrote: >> On Thu, Apr 27, 2017 at 09:34:04AM -0500, Eric Sandeen wrote: >>> On 4/27/17 3:39 AM, Lukas Czerner wrote: >>>> fstrim can take really long time on big, slow device or on file system >>>> with a lots of allocation groups. Currently there is no way for the user >>>> to cancell the operation. This patch makes it possible for the user to >>>> kill fstrim pocess by adding the check for fatal_signal_pending() in >>>> xfs_trim_extents(). >>> >>> With this patch, it seems that if it is killed, then nothing is copied >>> back to the user about the number of blocks trimmed before the kill. >>> >>> Is that intentional? >>> >>> I think it could be done in a way that the progress until the kill >>> does get reported, and that might be more useful? >> >> AFAICT ext4 doesn't report anything if somoene ERESTARTSYS's it... > > Well we aren't ext4, are we? ;) > > actually, it seems that it may report "count" as ERESTARTSYS in: > > ext4_debug("trimmed %d blocks in the group %d\n", > count, group); > > But yeah, although it does update the &range values, they don't > get copied out on any error. > > Anyway, ok, ext4 doesn't. Is there a reason that we shouldn't? Ok I am probably misunderstanding how things work, maybe it's not actually possible (he says after a little experimentation.) Feel free to ignore me on this one. :) -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/27/17 3:39 AM, Lukas Czerner wrote: > fstrim can take really long time on big, slow device or on file system > with a lots of allocation groups. Currently there is no way for the user > to cancell the operation. This patch makes it possible for the user to > kill fstrim pocess by adding the check for fatal_signal_pending() in > xfs_trim_extents(). > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> Sorry for the detour while I got confused about signals. ;) This looks ok to me. Thanks! Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > fs/xfs/xfs_discard.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > index d796ffa..6a05d27 100644 > --- a/fs/xfs/xfs_discard.c > +++ b/fs/xfs/xfs_discard.c > @@ -132,6 +132,11 @@ xfs_trim_extents( > error = xfs_btree_decrement(cur, 0, &i); > if (error) > goto out_del_cursor; > + > + if (fatal_signal_pending(current)) { > + error = -ERESTARTSYS; > + goto out_del_cursor; > + } > } > > out_del_cursor: > @@ -196,8 +201,11 @@ xfs_ioc_trim( > for (agno = start_agno; agno <= end_agno; agno++) { > error = xfs_trim_extents(mp, agno, start, end, minlen, > &blocks_trimmed); > - if (error) > + if (error) { > last_error = error; > + if (error == -ERESTARTSYS) > + break; > + } > } > > if (last_error) > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 27, 2017 at 10:39:28AM +0200, Lukas Czerner wrote: > fstrim can take really long time on big, slow device or on file system > with a lots of allocation groups. Currently there is no way for the user > to cancell the operation. This patch makes it possible for the user to > kill fstrim pocess by adding the check for fatal_signal_pending() in > xfs_trim_extents(). > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_discard.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > index d796ffa..6a05d27 100644 > --- a/fs/xfs/xfs_discard.c > +++ b/fs/xfs/xfs_discard.c > @@ -132,6 +132,11 @@ xfs_trim_extents( > error = xfs_btree_decrement(cur, 0, &i); > if (error) > goto out_del_cursor; > + > + if (fatal_signal_pending(current)) { > + error = -ERESTARTSYS; > + goto out_del_cursor; > + } > } > > out_del_cursor: > @@ -196,8 +201,11 @@ xfs_ioc_trim( > for (agno = start_agno; agno <= end_agno; agno++) { > error = xfs_trim_extents(mp, agno, start, end, minlen, > &blocks_trimmed); > - if (error) > + if (error) { > last_error = error; > + if (error == -ERESTARTSYS) > + break; > + } > } > > if (last_error) > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 27, 2017 at 10:50:02AM -0500, Eric Sandeen wrote: > On 4/27/17 10:46 AM, Eric Sandeen wrote: > > On 4/27/17 10:38 AM, Darrick J. Wong wrote: > >> On Thu, Apr 27, 2017 at 09:34:04AM -0500, Eric Sandeen wrote: > >>> On 4/27/17 3:39 AM, Lukas Czerner wrote: > >>>> fstrim can take really long time on big, slow device or on file system > >>>> with a lots of allocation groups. Currently there is no way for the user > >>>> to cancell the operation. This patch makes it possible for the user to > >>>> kill fstrim pocess by adding the check for fatal_signal_pending() in > >>>> xfs_trim_extents(). > >>> > >>> With this patch, it seems that if it is killed, then nothing is copied > >>> back to the user about the number of blocks trimmed before the kill. > >>> > >>> Is that intentional? > >>> > >>> I think it could be done in a way that the progress until the kill > >>> does get reported, and that might be more useful? > >> > >> AFAICT ext4 doesn't report anything if somoene ERESTARTSYS's it... > > > > Well we aren't ext4, are we? ;) > > > > actually, it seems that it may report "count" as ERESTARTSYS in: > > > > ext4_debug("trimmed %d blocks in the group %d\n", > > count, group); > > > > But yeah, although it does update the &range values, they don't > > get copied out on any error. > > > > Anyway, ok, ext4 doesn't. Is there a reason that we shouldn't? > > Ok I am probably misunderstanding how things work, maybe it's > not actually possible (he says after a little experimentation.) > Feel free to ignore me on this one. :) > > -Eric Hi Eric, I agree with Darrick, at the moment fstrim is not sticikng around to care, I am not even sure if it can stick around to care. Thanks! -Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index d796ffa..6a05d27 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -132,6 +132,11 @@ xfs_trim_extents( error = xfs_btree_decrement(cur, 0, &i); if (error) goto out_del_cursor; + + if (fatal_signal_pending(current)) { + error = -ERESTARTSYS; + goto out_del_cursor; + } } out_del_cursor: @@ -196,8 +201,11 @@ xfs_ioc_trim( for (agno = start_agno; agno <= end_agno; agno++) { error = xfs_trim_extents(mp, agno, start, end, minlen, &blocks_trimmed); - if (error) + if (error) { last_error = error; + if (error == -ERESTARTSYS) + break; + } } if (last_error)
fstrim can take really long time on big, slow device or on file system with a lots of allocation groups. Currently there is no way for the user to cancell the operation. This patch makes it possible for the user to kill fstrim pocess by adding the check for fatal_signal_pending() in xfs_trim_extents(). Signed-off-by: Lukas Czerner <lczerner@redhat.com> Reported-by: Zdenek Kabelac <zkabelac@redhat.com> --- fs/xfs/xfs_discard.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)