Re: [PATCH 2/2] xfs: add 'discard_sync' mount flag

Jens Axboe
2018-04-30T23:00:14Z

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