[PATCH] block: Make blkdev_issue_discard() interruptible

[PATCH] block: Make blkdev_issue_discard() interruptible

[Lukas Czerner]

Since the discard may take quite long time, especially with really big
extents (like the whole device for example), it would be nice to give to
users the opportunity to abort it. This is especially useful for mkfs,
when user can not know in advance how long it will take.

In conjunction with mke2fs patch "Inform user about ongoing discard"
it gives the user all the comfort of being informed about discard and
being able to abort the operation.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 block/blk-lib.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index d0216b9..4f54a1a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -102,6 +102,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		else if (!bio_flagged(bio, BIO_UPTODATE))
 			ret = -EIO;
 		bio_put(bio);
+
+		if (signal_pending(current)) {
+			ret = -ERESTARTSYS;
+			break;
+		}
 	}
 	return ret;
 out_free_page:
-- 
1.7.2.2

Re: [PATCH] block: Make blkdev_issue_discard() interruptible

[Mike Snitzer]

On Wed, Sep 15, 2010 at 10:32 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> Since the discard may take quite long time, especially with really big
> extents (like the whole device for example), it would be nice to give to
> users the opportunity to abort it. This is especially useful for mkfs,
> when user can not know in advance how long it will take.
>
> In conjunction with mke2fs patch "Inform user about ongoing discard"
> it gives the user all the comfort of being informed about discard and
> being able to abort the operation.
>
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  block/blk-lib.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index d0216b9..4f54a1a 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -102,6 +102,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
>                else if (!bio_flagged(bio, BIO_UPTODATE))
>                        ret = -EIO;
>                bio_put(bio);
> +
> +               if (signal_pending(current)) {
> +                       ret = -ERESTARTSYS;
> +                       break;
> +               }
>        }
>        return ret;
>  out_free_page:

Neil Brown recently suggested the use of fatal_signal_pending() rather
than signal_epnding() in another thread:
http://lkml.org/lkml/2010/9/12/232

I think Neil's suggestion applies here too?

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] block: Make blkdev_issue_discard() interruptible

[Lukas Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1622 bytes --]

On Wed, 15 Sep 2010, Mike Snitzer wrote:

> On Wed, Sep 15, 2010 at 10:32 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> > Since the discard may take quite long time, especially with really big
> > extents (like the whole device for example), it would be nice to give to
> > users the opportunity to abort it. This is especially useful for mkfs,
> > when user can not know in advance how long it will take.
> >
> > In conjunction with mke2fs patch "Inform user about ongoing discard"
> > it gives the user all the comfort of being informed about discard and
> > being able to abort the operation.
> >
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> >  block/blk-lib.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >
> > diff --git a/block/blk-lib.c b/block/blk-lib.c
> > index d0216b9..4f54a1a 100644
> > --- a/block/blk-lib.c
> > +++ b/block/blk-lib.c
> > @@ -102,6 +102,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> >                else if (!bio_flagged(bio, BIO_UPTODATE))
> >                        ret = -EIO;
> >                bio_put(bio);
> > +
> > +               if (signal_pending(current)) {
> > +                       ret = -ERESTARTSYS;
> > +                       break;
> > +               }
> >        }
> >        return ret;
> >  out_free_page:
> 
> Neil Brown recently suggested the use of fatal_signal_pending() rather
> than signal_epnding() in another thread:
> http://lkml.org/lkml/2010/9/12/232
> 
> I think Neil's suggestion applies here too?
> 
> Mike

Good to know, thanks a lot. I'll resend the patch shortly.

-Lukas

[PATCH] block: Make blkdev_issue_discard() interruptible

[Lukas Czerner]

Since the discard may take quite long time, especially with really big
extents (like the whole device for example), it would be nice to give to
users the opportunity to abort it. This is especially useful for mkfs,
when user can not know in advance how long it will take.

In conjunction with mke2fs patch "Inform user about ongoing discard"
it gives the user all the comfort of being informed about discard and
being able to abort the operation.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 block/blk-lib.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index c392029..204839b 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -101,6 +101,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		else if (!bio_flagged(bio, BIO_UPTODATE))
 			ret = -EIO;
 		bio_put(bio);
+
+		if (fatal_signal_pending(current)) {
+			ret = -ERESTARTSYS;
+			break;
+		}
 	}
 
 	return ret;
-- 
1.7.2.3

[PATCH] block: Make blkdev_issue_discard() interruptible

[Lukas Czerner]

Since the discard may take quite long time, especially with really big
extents (like the whole device for example), it would be nice to give to
users the opportunity to abort it. This is especially useful for mkfs,
when user can not know in advance how long it will take.

In conjunction with mke2fs patch "Inform user about ongoing discard"
it gives the user all the comfort of being informed about discard and
being able to abort the operation.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 block/blk-lib.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 1a320d2..db44671 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -91,6 +91,9 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		bio_get(bio);
 		submit_bio(type, bio);
 
+		if (unlikely(fatal_signal_pending(current)))
+			ret = -ERESTARTSYS;
+
 		wait_for_completion(&wait);
 
 		if (bio_flagged(bio, BIO_EOPNOTSUPP))
-- 
1.7.2.3

Re: [PATCH] block: Make blkdev_issue_discard() interruptible

[Jeff Moyer]

Lukas Czerner <lczerner@redhat.com> writes:

> Since the discard may take quite long time, especially with really big
> extents (like the whole device for example), it would be nice to give to
> users the opportunity to abort it. This is especially useful for mkfs,
> when user can not know in advance how long it will take.
>
> In conjunction with mke2fs patch "Inform user about ongoing discard"
> it gives the user all the comfort of being informed about discard and
> being able to abort the operation.

For others reviewing, this is against the block tree's for-next branch.

> ---
>  block/blk-lib.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 1a320d2..db44671 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -91,6 +91,9 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
>  		bio_get(bio);
>  		submit_bio(type, bio);
>  
> +		if (unlikely(fatal_signal_pending(current)))
> +			ret = -ERESTARTSYS;
> +
>  		wait_for_completion(&wait);
>  
>  		if (bio_flagged(bio, BIO_EOPNOTSUPP))

Given that you're still doing the wait_for_completion, wouldn't it be
better to check for a pending signal after that?

What testing did you perform?

Cheers,
Jeff

[PATCH] block: Make blkdev_issue_discard() interruptible

[Lukas Czerner]

Since the discard may take quite long time, especially with really big
extents (like the whole device for example), it would be nice to give to
users the opportunity to abort it. This is especially useful for mkfs,
when user can not know in advance how long it will take.

In conjunction with mke2fs patch "Inform user about ongoing discard"
it gives the user all the comfort of being informed about discard and
being able to abort the operation.

It has been tested on SSD device where mkfs.ext4 without -K takes rally
long due to discarding whole device. With this patch I am able to
successfully terminate that with ctrl+c. Aside that discard functionality
remains unchanged.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 block/blk-lib.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index c392029..54994e7 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -101,6 +101,9 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 		else if (!bio_flagged(bio, BIO_UPTODATE))
 			ret = -EIO;
 		bio_put(bio);
+
+		if (unlikely(fatal_signal_pending(current)))
+			ret = -ERESTARTSYS;
 	}
 
 	return ret;
-- 
1.7.2.3