On 2019/11/12 13:05, Damien Le Moal wrote: > On 2019/11/08 20:54, Tetsuo Handa wrote: >> syzbot found that a thread can stall for minutes inside fallocate() >> after that thread was killed by SIGKILL [1]. While trying to allocate >> 64TB of disk space using fallocate() is legal, delaying termination of >> killed thread for minutes is bad. Thus, allow iteration functions in >> block/blk-lib.c to be killable. >> >> [1] https://syzkaller.appspot.com/bug?id=9386d051e11e09973d5a4cf79af5e8cedf79386d >> >> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> >> Reported-by: syzbot <syzbot+b48daca8639150bc5e73@xxxxxxxxxxxxxxxxxxxxxxxxx> >> --- >> block/blk-lib.c | 44 ++++++++++++++++++++++++++++++++++++++++---- >> 1 file changed, 40 insertions(+), 4 deletions(-) >> >> diff --git a/block/blk-lib.c b/block/blk-lib.c >> index 5f2c429..6ca7cae 100644 >> --- a/block/blk-lib.c >> +++ b/block/blk-lib.c >> @@ -7,9 +7,22 @@ >> #include <linux/bio.h> >> #include <linux/blkdev.h> >> #include <linux/scatterlist.h> >> +#include <linux/sched/signal.h> >> >> #include "blk.h" >> >> +static int blk_should_abort(struct bio *bio) >> +{ >> + int ret; >> + >> + cond_resched(); >> + if (!fatal_signal_pending(current)) >> + return 0; >> + ret = submit_bio_wait(bio); > > This will change the behavior of __blkdev_issue_discard() to a sync IO > execution instead of the current async execution since submit_bio_wait() > call is the responsibility of the caller (e.g. blkdev_issue_discard()). > Have you checked if users of __blkdev_issue_discard() are OK with that ? > f2fs, ext4, xfs, dm and nvme use this function. I'm not sure... > > Looking at f2fs, this does not look like it is going to work as expected > since the bio setup, including end_io callback, is done after this > function is called and a regular submit_bio() execution is being used. Then, just breaking the iteration like below? nvmet_bdev_execute_write_zeroes() ignores -EINTR if "*biop = bio;" is done. Is that no problem? --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -7,6 +7,7 @@ #include <linux/bio.h> #include <linux/blkdev.h> #include <linux/scatterlist.h> +#include <linux/sched/signal.h> #include "blk.h" @@ -30,6 +31,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, struct bio *bio = *biop; unsigned int op; sector_t bs_mask; + int ret = 0; if (!q) return -ENXIO; @@ -76,10 +78,14 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, * is disabled. */ cond_resched(); + if (fatal_signal_pending(current)) { + ret = -EINTR; + break; + } } *biop = bio; - return 0; + return ret; } EXPORT_SYMBOL(__blkdev_issue_discard); @@ -136,6 +142,7 @@ static int __blkdev_issue_write_same(struct block_device *bdev, sector_t sector, unsigned int max_write_same_sectors; struct bio *bio = *biop; sector_t bs_mask; + int ret = 0; if (!q) return -ENXIO; @@ -172,10 +179,14 @@ static int __blkdev_issue_write_same(struct block_device *bdev, sector_t sector, nr_sects = 0; } cond_resched(); + if (fatal_signal_pending(current)) { + ret = -EINTR; + break; + } } *biop = bio; - return 0; + return ret; } /** @@ -216,6 +227,7 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev, struct bio *bio = *biop; unsigned int max_write_zeroes_sectors; struct request_queue *q = bdev_get_queue(bdev); + int ret = 0; if (!q) return -ENXIO; @@ -246,10 +258,14 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev, nr_sects = 0; } cond_resched(); + if (fatal_signal_pending(current)) { + ret = -EINTR; + break; + } } *biop = bio; - return 0; + return ret; } /* @@ -273,6 +289,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 (!q) return -ENXIO; @@ -296,10 +313,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; } /** > >> + bio_put(bio); >> + return ret ? ret : -EINTR; >> +} >> + >> struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp) >> { >> struct bio *new = bio_alloc(gfp, nr_pages);
