* [PATCH] xfs: Allow user to kill fstrim process
@ 2017-04-27 8:39 Lukas Czerner
2017-04-27 14:34 ` Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Lukas Czerner @ 2017-04-27 8:39 UTC (permalink / raw)
To: linux-xfs; +Cc: Lukas Czerner
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 8:39 [PATCH] xfs: Allow user to kill fstrim process Lukas Czerner
@ 2017-04-27 14:34 ` Eric Sandeen
2017-04-27 15:38 ` Darrick J. Wong
2017-04-27 15:55 ` Eric Sandeen
2017-04-27 21:04 ` Darrick J. Wong
2 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2017-04-27 14:34 UTC (permalink / raw)
To: Lukas Czerner, linux-xfs
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)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 14:34 ` Eric Sandeen
@ 2017-04-27 15:38 ` Darrick J. Wong
2017-04-27 15:46 ` Eric Sandeen
0 siblings, 1 reply; 8+ messages in thread
From: Darrick J. Wong @ 2017-04-27 15:38 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Lukas Czerner, linux-xfs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 15:38 ` Darrick J. Wong
@ 2017-04-27 15:46 ` Eric Sandeen
2017-04-27 15:50 ` Eric Sandeen
0 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2017-04-27 15:46 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Lukas Czerner, linux-xfs
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 15:46 ` Eric Sandeen
@ 2017-04-27 15:50 ` Eric Sandeen
2017-04-28 8:22 ` Lukas Czerner
0 siblings, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2017-04-27 15:50 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Lukas Czerner, linux-xfs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 8:39 [PATCH] xfs: Allow user to kill fstrim process Lukas Czerner
2017-04-27 14:34 ` Eric Sandeen
@ 2017-04-27 15:55 ` Eric Sandeen
2017-04-27 21:04 ` Darrick J. Wong
2 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2017-04-27 15:55 UTC (permalink / raw)
To: Lukas Czerner, linux-xfs
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)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 8:39 [PATCH] xfs: Allow user to kill fstrim process Lukas Czerner
2017-04-27 14:34 ` Eric Sandeen
2017-04-27 15:55 ` Eric Sandeen
@ 2017-04-27 21:04 ` Darrick J. Wong
2 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2017-04-27 21:04 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Allow user to kill fstrim process
2017-04-27 15:50 ` Eric Sandeen
@ 2017-04-28 8:22 ` Lukas Czerner
0 siblings, 0 replies; 8+ messages in thread
From: Lukas Czerner @ 2017-04-28 8:22 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Darrick J. Wong, linux-xfs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-28 8:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-27 8:39 [PATCH] xfs: Allow user to kill fstrim process Lukas Czerner
2017-04-27 14:34 ` Eric Sandeen
2017-04-27 15:38 ` Darrick J. Wong
2017-04-27 15:46 ` Eric Sandeen
2017-04-27 15:50 ` Eric Sandeen
2017-04-28 8:22 ` Lukas Czerner
2017-04-27 15:55 ` Eric Sandeen
2017-04-27 21:04 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).