On 2/23/24 00:49, Keith Busch wrote:
> From: Keith Busch <kbusch@xxxxxxxxxx>
>
> Some of these block operations can access a significant capacity and
> take longer than the user expected. A user may change their mind about
> wanting to run that command and attempt to kill the process and do
> something else with their device. But since the task is uninterruptable,
> they have to wait for it to finish, which could be many hours.
>
> Check for a fatal signal at each iteration so the user doesn't have to
> wait for their regretted operation to complete naturally.
>
> Reported-by: Conrad Meyer <conradmeyer@xxxxxxxx>
> Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> ---
> block/blk-lib.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index d4c476cf3784a..9e594f641ce72 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -35,6 +35,23 @@ static sector_t bio_discard_limit(struct block_device *bdev, sector_t sector)
> return round_down(UINT_MAX, discard_granularity) >> SECTOR_SHIFT;
> }
>
> +static void abort_bio_endio(struct bio *bio)
> +{
> + complete(bio->bi_private);
> + bio_put(bio);
> +}
> +
> +static void abort_bio(struct bio *bio)
> +{
> + DECLARE_COMPLETION_ONSTACK_MAP(done,
> + bio->bi_bdev->bd_disk->lockdep_map);
> +
> + bio->bi_private = &done;
> + bio->bi_end_io = abort_bio_endio;
> + bio_endio(bio);
> + blk_wait_io(&done);
> +}
> +
> @@ -143,6 +164,10 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev,
> nr_sects -= len;
> sector += len;
> cond_resched();
> + if (fatal_signal_pending(current)) {
> + abort_bio(bio);
> + return -EINTR;
> + }
> }
>
> *biop = bio;
> @@ -187,6 +212,10 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
> break;
> }
> cond_resched();
> + if (fatal_signal_pending(current)) {
> + abort_bio(bio);
> + return -EINTR;
> + }
> }
>
If a device with large capacity supports write zero offload and user kills that
long outstanding write zero operation then it appears we run through the fatal_signal_pending()
and abort_bio() twice: once under __blkdev_issue_write_zeroes() and then latter under
__blkdev_issue_zero_pages(). The entry to __blkdev_issue_zero_pages() happens if
__blkdev_issue_write_zeroes() returns the error code and BLKDEV_ZERO_NOFALLBACK is NOT
specified in flags.
I think if fatal signal is intercepted while running __blkdev_issue_write_zeroes() then we
shouldn't need to re-enter the __blkdev_issue_zero_pages(). We may want to add following code:
@@ -280,7 +306,7 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
bio_put(bio);
}
blk_finish_plug(&plug);
- if (ret && try_write_zeroes) {
+ if (ret && ret != -EINTR && try_write_zeroes) {
if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
try_write_zeroes = false;
goto retry;
Thanks,
--Nilay
