On 2/20/24 12:41, Keith Busch wrote:
> From: Keith Busch <kbusch@xxxxxxxxxx>
>
> If a user runs something like `blkdiscard -z /dev/sda`, and the device
> does not have an efficient write zero offload, the kernel will dispatch
> long chains of bio's using the ZERO_PAGE for the entire capacity of the
> device. If the capacity is very large, this process could take a long
> time in an uninterruptable state, which the user may want to abort.
> Check between batches for the user's request to kill the process so they
> don't need to wait potentially many hours.
>
> Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> ---
> block/blk-lib.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index e59c3069e8351..d5c334aa98e0d 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -190,6 +190,8 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
> break;
> }
> cond_resched();
> + if (fatal_signal_pending(current))
> + break;
> }
>
> *biop = bio;
We are exiting before completion of the whole I/O request, does it makes
sense to return 0 == success if I/O is interrupted by the signal ?
that means I/O not completed, hence it is actually an error, can we return
the -EINTR when you are handling outstanding signal ?
something like this untested :-
linux-block (for-next) # git diff
diff --git a/block/blk-lib.c b/block/blk-lib.c
index e59c3069e835..bfadd9eecc6e 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -172,6 +172,7 @@ static int __blkdev_issue_zero_pages(struct
block_device *bdev,
struct bio *bio = *biop;
int bi_size = 0;
unsigned int sz;
+ int ret = 0;
if (bdev_read_only(bdev))
return -EPERM;
@@ -190,10 +191,14 @@ static int __blkdev_issue_zero_pages(struct
block_device *bdev,
break;
}
cond_resched();
+ if (fatal_signal_pending(current)) {
+ ret = -EINTR;
+ break;
+ }
}
*biop = bio;
- return 0;
+ return ret;
}
/**
-ck
