On 4/30/18 4:40 PM, Jens Axboe wrote: > On 4/30/18 4:28 PM, Dave Chinner wrote: >> Yes, it does, but so would having the block layer to throttle device >> discard requests in flight to a queue depth of 1. And then we don't >> have to change XFS at all. > > I'm perfectly fine with making that change by default, and much easier > for me since I don't have to patch file systems. Totally untested, but this should do the trick. It ensures we have a QD of 1 (per caller), which should be sufficient. If people tune down the discard size, then you'll be blocking waiting for discards on issue. diff --git a/block/blk-lib.c b/block/blk-lib.c index a676084d4740..0bf9befcc863 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -11,16 +11,19 @@ #include "blk.h" static struct bio *next_bio(struct bio *bio, unsigned int nr_pages, - gfp_t gfp) + gfp_t gfp) { - struct bio *new = bio_alloc(gfp, nr_pages); - + /* + * Devices suck at discard, so if we have to break up the bio + * size due to the max discard size setting, wait for the + * previous one to finish first. + */ if (bio) { - bio_chain(bio, new); - submit_bio(bio); + submit_bio_wait(bio); + bio_put(bio); } - return new; + return bio_alloc(gfp, nr_pages); } int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, @@ -63,7 +66,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, sector_t end_sect, tmp; /* Make sure bi_size doesn't overflow */ - req_sects = min_t(sector_t, nr_sects, UINT_MAX >> 9); + req_sects = min_t(sector_t, nr_sects, + q->limits.max_discard_sectors); /** * If splitting a request, and the next starting sector would be -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html