* [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios
2013-11-25 22:30 [PATCH] Make generic_make_request() handle arbitrary size bios Kent Overstreet
@ 2013-11-25 22:30 ` Kent Overstreet
2013-11-26 6:09 ` Christoph Hellwig
2013-11-25 22:30 ` [PATCH 2/5] mtip32xx: handle arbitrary size bios Kent Overstreet
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Kent Overstreet @ 2013-11-25 22:30 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, axboe
Cc: Kent Overstreet, Neil Brown, Alasdair Kergon, dm-devel
The way the block layer is currently written, it goes to great lengths
to avoid having to split bios; upper layer code (such as bio_add_page())
checks what the underlying device can handle and tries to always create
bios that don't need to be split.
But this approach becomes unwieldy and eventually breaks down with
stacked devices and devices with dynamic limits, and it adds a lot of
complexity. If the block layer could split bios as needed, we could
eliminate a lot of complexity elsewhere - particularly in stacked
drivers. Code that creates bios can then create whatever size bios are
convenient, and more importantly stacked drivers don't have to deal with
both their own bio size limitations and the limitations of the
(potentially multiple) devices underneath them. In the future this will
let us delete merge_bvec_fn and a bunch of other code.
We do this by adding calls to blk_queue_split() to the various
make_request functions that need it - a few can already handle arbitrary
size bios. Note that we add the call _after_ any call to blk_queue_bounce();
this means that blk_queue_split() and blk_recalc_rq_segments() don't need to be
concerned with bouncing affecting segment merging.
Some make_request_fns were simple enough to audit and verify they don't
need blk_queue_split() calls. The skipped ones are:
* nfhd_make_request (arch/m68k/emu/nfblock.c)
* axon_ram_make_request (arch/powerpc/sysdev/axonram.c)
* simdisk_make_request (arch/xtensa/platforms/iss/simdisk.c)
* brd_make_request (ramdisk - drivers/block/brd.c)
* loop_make_request
* null_queue_bio
* bcache's make_request fns
Some others are almost certainly safe to remove now, but will be left for future
patches.
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Neil Brown <neilb@suse.de>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: dm-devel@redhat.com
---
block/blk-core.c | 19 ++--
block/blk-merge.c | 150 ++++++++++++++++++++++++++--
block/blk-mq.c | 2 +
drivers/block/drbd/drbd_req.c | 2 +
drivers/block/mtip32xx/mtip32xx.c | 6 +-
drivers/block/nvme-core.c | 2 +
drivers/block/pktcdvd.c | 6 +-
drivers/block/ps3vram.c | 2 +
drivers/block/rsxx/dev.c | 2 +
drivers/block/umem.c | 2 +
drivers/md/dm.c | 2 +
drivers/md/md.c | 2 +
drivers/s390/block/dcssblk.c | 2 +
drivers/s390/block/xpram.c | 2 +
drivers/staging/lustre/lustre/llite/lloop.c | 2 +
drivers/staging/zram/zram_drv.c | 2 +
include/linux/blkdev.h | 3 +
17 files changed, 185 insertions(+), 23 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5da8e90..5cc7171 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -566,6 +566,10 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
if (q->id < 0)
goto fail_c;
+ q->bio_split = bioset_create(4, 0);
+ if (!q->bio_split)
+ goto fail_id;
+
q->backing_dev_info.ra_pages =
(VM_MAX_READAHEAD * 1024) / PAGE_CACHE_SIZE;
q->backing_dev_info.state = 0;
@@ -575,7 +579,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
err = bdi_init(&q->backing_dev_info);
if (err)
- goto fail_id;
+ goto fail_split;
setup_timer(&q->backing_dev_info.laptop_mode_wb_timer,
laptop_mode_timer_fn, (unsigned long) q);
@@ -620,6 +624,8 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
fail_bdi:
bdi_destroy(&q->backing_dev_info);
+fail_split:
+ bioset_free(q->bio_split);
fail_id:
ida_simple_remove(&blk_queue_ida, q->id);
fail_c:
@@ -1472,6 +1478,8 @@ void blk_queue_bio(struct request_queue *q, struct bio *bio)
struct request *req;
unsigned int request_count = 0;
+ blk_queue_split(q, &bio, q->bio_split);
+
/*
* low level driver can indicate that it wants pages above a
* certain limit bounced to low memory (ie for highmem, or even
@@ -1694,15 +1702,6 @@ generic_make_request_checks(struct bio *bio)
goto end_io;
}
- if (likely(bio_is_rw(bio) &&
- nr_sectors > queue_max_hw_sectors(q))) {
- printk(KERN_ERR "bio too big device %s (%u > %u)\n",
- bdevname(bio->bi_bdev, b),
- bio_sectors(bio),
- queue_max_hw_sectors(q));
- goto end_io;
- }
-
part = bio->bi_bdev->bd_part;
if (should_fail_request(part, bio->bi_iter.bi_size) ||
should_fail_request(&part_to_disk(part)->part0,
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 05c17be..4a0ac34 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -9,11 +9,149 @@
#include "blk.h"
+static struct bio *blk_bio_discard_split(struct request_queue *q,
+ struct bio *bio,
+ struct bio_set *bs)
+{
+ unsigned int max_discard_sectors, granularity;
+ int alignment;
+ sector_t tmp;
+ unsigned split_sectors;
+
+ /* Zero-sector (unknown) and one-sector granularities are the same. */
+ granularity = max(q->limits.discard_granularity >> 9, 1U);
+
+ max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
+ max_discard_sectors -= max_discard_sectors % granularity;
+
+ if (unlikely(!max_discard_sectors)) {
+ /* XXX: warn */
+ return NULL;
+ }
+
+ if (bio_sectors(bio) <= max_discard_sectors)
+ return NULL;
+
+ split_sectors = max_discard_sectors;
+
+ /*
+ * If the next starting sector would be misaligned, stop the discard at
+ * the previous aligned sector.
+ */
+ alignment = (q->limits.discard_alignment >> 9) % granularity;
+
+ tmp = bio->bi_iter.bi_sector + split_sectors - alignment;
+ tmp = sector_div(tmp, granularity);
+
+ if (split_sectors > tmp)
+ split_sectors -= tmp;
+
+ return bio_split(bio, split_sectors, GFP_NOIO, bs);
+}
+
+static struct bio *blk_bio_write_same_split(struct request_queue *q,
+ struct bio *bio,
+ struct bio_set *bs)
+{
+ if (!q->limits.max_write_same_sectors)
+ return NULL;
+
+ if (bio_sectors(bio) <= q->limits.max_write_same_sectors)
+ return NULL;
+
+ return bio_split(bio, q->limits.max_write_same_sectors, GFP_NOIO, bs);
+}
+
+static struct bio *blk_bio_segment_split(struct request_queue *q,
+ struct bio *bio,
+ struct bio_set *bs)
+{
+ struct bio *split;
+ struct bio_vec bv, bvprv;
+ struct bvec_iter iter;
+ unsigned seg_size = 0, nsegs = 0;
+ int prev = 0;
+
+ struct bvec_merge_data bvm = {
+ .bi_bdev = bio->bi_bdev,
+ .bi_sector = bio->bi_iter.bi_sector,
+ .bi_size = 0,
+ .bi_rw = bio->bi_rw,
+ };
+
+ bio_for_each_segment(bv, bio, iter) {
+ if (q->merge_bvec_fn &&
+ q->merge_bvec_fn(q, &bvm, &bv) < (int) bv.bv_len)
+ goto split;
+
+ bvm.bi_size += bv.bv_len;
+
+ if (bvm.bi_size >> 9 > queue_max_sectors(q))
+ goto split;
+
+ if (prev && blk_queue_cluster(q)) {
+ if (seg_size + bv.bv_len > queue_max_segment_size(q))
+ goto new_segment;
+ if (!BIOVEC_PHYS_MERGEABLE(&bvprv, &bv))
+ goto new_segment;
+ if (!BIOVEC_SEG_BOUNDARY(q, &bvprv, &bv))
+ goto new_segment;
+
+ seg_size += bv.bv_len;
+ bvprv = bv;
+ prev = 1;
+ continue;
+ }
+new_segment:
+ if (nsegs == queue_max_segments(q))
+ goto split;
+
+ nsegs++;
+ bvprv = bv;
+ prev = 1;
+ seg_size = bv.bv_len;
+ }
+
+ return NULL;
+split:
+ split = bio_clone_bioset(bio, GFP_NOIO, bs);
+
+ split->bi_iter.bi_size -= iter.bi_size;
+ bio->bi_iter = iter;
+
+ if (bio_integrity(bio)) {
+ bio_integrity_advance(bio, split->bi_iter.bi_size);
+ bio_integrity_trim(split, 0, bio_sectors(split));
+ }
+
+ return split;
+}
+
+void blk_queue_split(struct request_queue *q, struct bio **bio,
+ struct bio_set *bs)
+{
+ struct bio *split;
+
+ if ((*bio)->bi_rw & REQ_DISCARD)
+ split = blk_bio_discard_split(q, *bio, bs);
+ else if ((*bio)->bi_rw & REQ_WRITE_SAME)
+ split = blk_bio_write_same_split(q, *bio, bs);
+ else
+ split = blk_bio_segment_split(q, *bio, q->bio_split);
+
+ if (split) {
+ bio_chain(split, *bio);
+ generic_make_request(*bio);
+ *bio = split;
+ }
+}
+EXPORT_SYMBOL(blk_queue_split);
+
static unsigned int __blk_recalc_rq_segments(struct request_queue *q,
struct bio *bio)
{
struct bio_vec bv, bvprv = { NULL };
- int cluster, high, highprv = 1;
+ int cluster, prev = 0;
unsigned int seg_size, nr_phys_segs;
struct bio *fbio, *bbio;
struct bvec_iter iter;
@@ -27,13 +165,7 @@ static unsigned int __blk_recalc_rq_segments(struct request_queue *q,
nr_phys_segs = 0;
for_each_bio(bio) {
bio_for_each_segment(bv, bio, iter) {
- /*
- * the trick here is making sure that a high page is
- * never considered part of another segment, since that
- * might change with the bounce page.
- */
- high = page_to_pfn(bv.bv_page) > queue_bounce_pfn(q);
- if (!high && !highprv && cluster) {
+ if (prev && cluster) {
if (seg_size + bv.bv_len
> queue_max_segment_size(q))
goto new_segment;
@@ -53,8 +185,8 @@ new_segment:
nr_phys_segs++;
bvprv = bv;
+ prev = 1;
seg_size = bv.bv_len;
- highprv = high;
}
bbio = bio;
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index e4fbcc3..d29be09 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -912,6 +912,8 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
blk_queue_bounce(q, &bio);
+ blk_queue_split(q, &bio, q->bio_split);
+
if (use_plug && blk_attempt_plug_merge(q, bio, &request_count))
return;
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 104a040..941a69c 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -1275,6 +1275,8 @@ void drbd_make_request(struct request_queue *q, struct bio *bio)
struct drbd_conf *mdev = (struct drbd_conf *) q->queuedata;
unsigned long start_time;
+ blk_queue_split(q, &bio, q->bio_split);
+
start_time = jiffies;
/*
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 52b2f2a..1b3dfee 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -3967,6 +3967,10 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio)
int nents = 0;
int tag = 0, unaligned = 0;
+ blk_queue_bounce(queue, &bio);
+
+ blk_queue_split(queue, &bio, q->bio_split);
+
if (unlikely(dd->dd_flag & MTIP_DDF_STOP_IO)) {
if (unlikely(test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
&dd->dd_flag))) {
@@ -4016,8 +4020,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio)
sg = mtip_hw_get_scatterlist(dd, &tag, unaligned);
if (likely(sg != NULL)) {
- blk_queue_bounce(queue, &bio);
-
if (unlikely((bio)->bi_vcnt > MTIP_MAX_SG)) {
dev_warn(&dd->pdev->dev,
"Maximum number of SGL entries exceeded\n");
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 1f14ac4..cfb8f7b 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -707,6 +707,8 @@ static void nvme_make_request(struct request_queue *q, struct bio *bio)
struct nvme_queue *nvmeq = get_nvmeq(ns->dev);
int result = -EBUSY;
+ blk_queue_split(q, &bio, q->bio_split);
+
if (!nvmeq) {
put_nvmeq(NULL);
bio_endio(bio, -EIO);
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 3dda09a..38799f8 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2442,6 +2442,10 @@ static void pkt_make_request(struct request_queue *q, struct bio *bio)
char b[BDEVNAME_SIZE];
struct bio *split;
+ blk_queue_bounce(q, &bio);
+
+ blk_queue_split(q, &bio, q->bio_split);
+
pd = q->queuedata;
if (!pd) {
pr_err("%s incorrect request queue\n",
@@ -2472,8 +2476,6 @@ static void pkt_make_request(struct request_queue *q, struct bio *bio)
goto end_io;
}
- blk_queue_bounce(q, &bio);
-
do {
sector_t zone = get_zone(bio->bi_iter.bi_sector, pd);
sector_t last_zone = get_zone(bio_end_sector(bio) - 1, pd);
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index ef45cfb..a995972 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -603,6 +603,8 @@ static void ps3vram_make_request(struct request_queue *q, struct bio *bio)
struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev);
int busy;
+ blk_queue_split(q, &bio, q->bio_split);
+
dev_dbg(&dev->core, "%s\n", __func__);
spin_lock_irq(&priv->lock);
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index 2839d37..ff074a3 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -169,6 +169,8 @@ static void rsxx_make_request(struct request_queue *q, struct bio *bio)
struct rsxx_bio_meta *bio_meta;
int st = -EINVAL;
+ blk_queue_split(q, &bio, q->bio_split);
+
might_sleep();
if (!card)
diff --git a/drivers/block/umem.c b/drivers/block/umem.c
index 4cf81b5..13d577c 100644
--- a/drivers/block/umem.c
+++ b/drivers/block/umem.c
@@ -531,6 +531,8 @@ static void mm_make_request(struct request_queue *q, struct bio *bio)
(unsigned long long)bio->bi_iter.bi_sector,
bio->bi_iter.bi_size);
+ blk_queue_split(q, &bio, q->bio_split);
+
spin_lock_irq(&card->lock);
*card->biotail = bio;
bio->bi_next = NULL;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 44a2fa6..0bfb050 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1500,6 +1500,8 @@ static void dm_request(struct request_queue *q, struct bio *bio)
{
struct mapped_device *md = q->queuedata;
+ blk_queue_split(q, &bio, q->bio_split);
+
if (dm_request_based(md))
blk_queue_bio(q, bio);
else
diff --git a/drivers/md/md.c b/drivers/md/md.c
index b07fed3..de39a8a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -256,6 +256,8 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
int cpu;
unsigned int sectors;
+ blk_queue_split(q, &bio, q->bio_split);
+
if (mddev == NULL || mddev->pers == NULL
|| !mddev->ready) {
bio_io_error(bio);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index ebf41e2..db33cd3 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -815,6 +815,8 @@ dcssblk_make_request(struct request_queue *q, struct bio *bio)
unsigned long source_addr;
unsigned long bytes_done;
+ blk_queue_split(q, &bio, q->bio_split);
+
bytes_done = 0;
dev_info = bio->bi_bdev->bd_disk->private_data;
if (dev_info == NULL)
diff --git a/drivers/s390/block/xpram.c b/drivers/s390/block/xpram.c
index 3e530f9..3ae29a0 100644
--- a/drivers/s390/block/xpram.c
+++ b/drivers/s390/block/xpram.c
@@ -190,6 +190,8 @@ static void xpram_make_request(struct request_queue *q, struct bio *bio)
unsigned long page_addr;
unsigned long bytes;
+ blk_queue_split(q, &bio, q->bio_split);
+
if ((bio->bi_iter.bi_sector & 7) != 0 ||
(bio->bi_iter.bi_size & 4095) != 0)
/* Request is not page-aligned. */
diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index 581ff78..776d44c 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -344,6 +344,8 @@ static void loop_make_request(struct request_queue *q, struct bio *old_bio)
int rw = bio_rw(old_bio);
int inactive;
+ blk_queue_split(q, &old_bio, q->bio_split);
+
if (!lo)
goto err;
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index 6f98838..56eb09e 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -734,6 +734,8 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio)
{
struct zram *zram = queue->queuedata;
+ blk_queue_split(queue, &bio, queue->bio_split);
+
down_read(&zram->init_lock);
if (unlikely(!zram->init_done))
goto error;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 02cb6f0..88d4e7b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -476,6 +476,7 @@ struct request_queue {
wait_queue_head_t mq_freeze_wq;
struct percpu_counter mq_usage_counter;
struct list_head all_q_node;
+ struct bio_set *bio_split;
};
#define QUEUE_FLAG_QUEUED 1 /* uses generic tag queueing */
@@ -787,6 +788,8 @@ extern void blk_rq_unprep_clone(struct request *rq);
extern int blk_insert_cloned_request(struct request_queue *q,
struct request *rq);
extern void blk_delay_queue(struct request_queue *, unsigned long);
+extern void blk_queue_split(struct request_queue *, struct bio **,
+ struct bio_set *);
extern void blk_recount_segments(struct request_queue *, struct bio *);
extern int scsi_verify_blk_ioctl(struct block_device *, unsigned int);
extern int scsi_cmd_blk_ioctl(struct block_device *, fmode_t,
--
1.8.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios
2013-11-25 22:30 ` [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios Kent Overstreet
@ 2013-11-26 6:09 ` Christoph Hellwig
2013-11-26 7:21 ` Kent Overstreet
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2013-11-26 6:09 UTC (permalink / raw)
To: Kent Overstreet
Cc: linux-kernel, linux-fsdevel, axboe, Neil Brown, Alasdair Kergon,
dm-devel
> + q->bio_split = bioset_create(4, 0);
> + if (!q->bio_split)
> + goto fail_id;
How did we arrive at a mempool size of 4 to make sure we can always make
progress with arbitrarily sized bios? Shouldn't we document the design
decision somewhere?
> +static struct bio *blk_bio_discard_split(struct request_queue *q,
> + struct bio *bio,
> + struct bio_set *bs)
> +{
One fallout of this should be that it would allows us to impement
a range UNMAP/TRIP much more easily, which would help dramatically
with TRIM performance!
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios
2013-11-26 6:09 ` Christoph Hellwig
@ 2013-11-26 7:21 ` Kent Overstreet
0 siblings, 0 replies; 12+ messages in thread
From: Kent Overstreet @ 2013-11-26 7:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-fsdevel, axboe, Neil Brown, Alasdair Kergon,
dm-devel
On Mon, Nov 25, 2013 at 10:09:21PM -0800, Christoph Hellwig wrote:
> > + q->bio_split = bioset_create(4, 0);
> > + if (!q->bio_split)
> > + goto fail_id;
>
> How did we arrive at a mempool size of 4 to make sure we can always make
> progress with arbitrarily sized bios? Shouldn't we document the design
> decision somewhere?
It just has to be nonzero to guarantee forward progress - the
bio_alloc_bioset() rescuer thing I did awhile back guarantees that.
> > +static struct bio *blk_bio_discard_split(struct request_queue *q,
> > + struct bio *bio,
> > + struct bio_set *bs)
> > +{
>
> One fallout of this should be that it would allows us to impement
> a range UNMAP/TRIP much more easily, which would help dramatically
> with TRIM performance!
Cool!
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] mtip32xx: handle arbitrary size bios
2013-11-25 22:30 [PATCH] Make generic_make_request() handle arbitrary size bios Kent Overstreet
2013-11-25 22:30 ` [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios Kent Overstreet
@ 2013-11-25 22:30 ` Kent Overstreet
2013-11-26 6:10 ` Christoph Hellwig
2013-11-25 22:30 ` [PATCH 3/5] block: Gut bio_add_page() Kent Overstreet
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Kent Overstreet @ 2013-11-25 22:30 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, axboe
Cc: Kent Overstreet, Asai Thambi S P, Sam Bradshaw
We get a measurable performance increase by handling this in the driver when
we're already looping over the biovec, instead of handling it separately in
generic_make_request() (or bio_add_page() originally)
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Asai Thambi S P <asamymuthupa@micron.com>
Cc: Sam Bradshaw <sbradshaw@micron.com>
---
drivers/block/mtip32xx/mtip32xx.c | 47 ++++++++++++---------------------------
1 file changed, 14 insertions(+), 33 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 1b3dfee..2b142eb 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2648,24 +2648,6 @@ static void mtip_hw_submit_io(struct driver_data *dd, sector_t sector,
}
/*
- * Release a command slot.
- *
- * @dd Pointer to the driver data structure.
- * @tag Slot tag
- *
- * return value
- * None
- */
-static void mtip_hw_release_scatterlist(struct driver_data *dd, int tag,
- int unaligned)
-{
- struct semaphore *sem = unaligned ? &dd->port->cmd_slot_unal :
- &dd->port->cmd_slot;
- release_slot(dd->port, tag);
- up(sem);
-}
-
-/*
* Obtain a command slot and return its associated scatter list.
*
* @dd Pointer to the driver data structure.
@@ -3969,8 +3951,6 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio)
blk_queue_bounce(queue, &bio);
- blk_queue_split(queue, &bio, q->bio_split);
-
if (unlikely(dd->dd_flag & MTIP_DDF_STOP_IO)) {
if (unlikely(test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
&dd->dd_flag))) {
@@ -4020,21 +4000,22 @@ static void mtip_make_request(struct request_queue *queue, struct bio *bio)
sg = mtip_hw_get_scatterlist(dd, &tag, unaligned);
if (likely(sg != NULL)) {
- if (unlikely((bio)->bi_vcnt > MTIP_MAX_SG)) {
- dev_warn(&dd->pdev->dev,
- "Maximum number of SGL entries exceeded\n");
- bio_io_error(bio);
- mtip_hw_release_scatterlist(dd, tag, unaligned);
- return;
- }
-
/* Create the scatter list for this bio. */
bio_for_each_segment(bvec, bio, iter) {
- sg_set_page(&sg[nents],
- bvec.bv_page,
- bvec.bv_len,
- bvec.bv_offset);
- nents++;
+ if (unlikely(nents == MTIP_MAX_SG)) {
+ struct bio *split = bio_clone(bio, GFP_NOIO);
+
+ split->bi_iter = iter;
+ bio->bi_iter.bi_size -= iter.bi_size;
+ bio_chain(split, bio);
+ generic_make_request(split);
+ break;
+ }
+
+ sg_set_page(&sg[nents++],
+ bvec.bv_page,
+ bvec.bv_len,
+ bvec.bv_offset);
}
/* Issue the read/write. */
--
1.8.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/5] mtip32xx: handle arbitrary size bios
2013-11-25 22:30 ` [PATCH 2/5] mtip32xx: handle arbitrary size bios Kent Overstreet
@ 2013-11-26 6:10 ` Christoph Hellwig
2013-11-26 7:32 ` Kent Overstreet
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2013-11-26 6:10 UTC (permalink / raw)
To: Kent Overstreet
Cc: linux-kernel, linux-fsdevel, axboe, Asai Thambi S P, Sam Bradshaw
On Mon, Nov 25, 2013 at 02:30:30PM -0800, Kent Overstreet wrote:
> We get a measurable performance increase by handling this in the driver when
> we're already looping over the biovec, instead of handling it separately in
> generic_make_request() (or bio_add_page() originally)
Given that Jens has a patch queue up to convert the driver to blk-mq
adjusting the blk-mq infrastructure to handle this might be a better
plan forward..
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] mtip32xx: handle arbitrary size bios
2013-11-26 6:10 ` Christoph Hellwig
@ 2013-11-26 7:32 ` Kent Overstreet
0 siblings, 0 replies; 12+ messages in thread
From: Kent Overstreet @ 2013-11-26 7:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-fsdevel, axboe, Asai Thambi S P, Sam Bradshaw
On Mon, Nov 25, 2013 at 10:10:18PM -0800, Christoph Hellwig wrote:
> On Mon, Nov 25, 2013 at 02:30:30PM -0800, Kent Overstreet wrote:
> > We get a measurable performance increase by handling this in the driver when
> > we're already looping over the biovec, instead of handling it separately in
> > generic_make_request() (or bio_add_page() originally)
>
> Given that Jens has a patch queue up to convert the driver to blk-mq
> adjusting the blk-mq infrastructure to handle this might be a better
> plan forward..
I mostly wanted the patch in the series as an example conversion -
converting blk-mq is going to take some thought and time to familiarize
myself with the code.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] block: Gut bio_add_page()
2013-11-25 22:30 [PATCH] Make generic_make_request() handle arbitrary size bios Kent Overstreet
2013-11-25 22:30 ` [PATCH 1/5] block: Make generic_make_request handle arbitrary sized bios Kent Overstreet
2013-11-25 22:30 ` [PATCH 2/5] mtip32xx: handle arbitrary size bios Kent Overstreet
@ 2013-11-25 22:30 ` Kent Overstreet
2013-11-25 22:30 ` [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now Kent Overstreet
2013-11-25 22:30 ` [PATCH 5/5] bcache: " Kent Overstreet
4 siblings, 0 replies; 12+ messages in thread
From: Kent Overstreet @ 2013-11-25 22:30 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, axboe; +Cc: Kent Overstreet
Since generic_make_request() can now handle arbitrary size bios, all we
have to do is make sure the bvec array doesn't overflow.
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Cc: Jens Axboe <axboe@kernel.dk>
---
fs/bio.c | 137 ++++++++++++++++++++++++++-------------------------------------
1 file changed, 57 insertions(+), 80 deletions(-)
diff --git a/fs/bio.c b/fs/bio.c
index 75c49a3..da27557 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -690,9 +690,23 @@ int bio_get_nr_vecs(struct block_device *bdev)
}
EXPORT_SYMBOL(bio_get_nr_vecs);
-static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page
- *page, unsigned int len, unsigned int offset,
- unsigned int max_sectors)
+/**
+ * bio_add_pc_page - attempt to add page to bio
+ * @q: the target queue
+ * @bio: destination bio
+ * @page: page to add
+ * @len: vec entry length
+ * @offset: vec entry offset
+ *
+ * Attempt to add a page to the bio_vec maplist. This can fail for a
+ * number of reasons, such as the bio being full or target block device
+ * limitations. The target block device must allow bio's up to PAGE_SIZE,
+ * so it is always possible to add a single page to an empty bio.
+ *
+ * This should only be used by REQ_PC bios.
+ */
+int bio_add_pc_page(struct request_queue *q, struct bio *bio, struct page
+ *page, unsigned int len, unsigned int offset)
{
int retried_segments = 0;
struct bio_vec *bvec;
@@ -703,7 +717,7 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page
if (unlikely(bio_flagged(bio, BIO_CLONED)))
return 0;
- if (((bio->bi_iter.bi_size + len) >> 9) > max_sectors)
+ if (((bio->bi_iter.bi_size + len) >> 9) > queue_max_hw_sectors(q))
return 0;
/*
@@ -716,28 +730,7 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page
if (page == prev->bv_page &&
offset == prev->bv_offset + prev->bv_len) {
- unsigned int prev_bv_len = prev->bv_len;
prev->bv_len += len;
-
- if (q->merge_bvec_fn) {
- struct bvec_merge_data bvm = {
- /* prev_bvec is already charged in
- bi_size, discharge it in order to
- simulate merging updated prev_bvec
- as new bvec. */
- .bi_bdev = bio->bi_bdev,
- .bi_sector = bio->bi_iter.bi_sector,
- .bi_size = bio->bi_iter.bi_size -
- prev_bv_len,
- .bi_rw = bio->bi_rw,
- };
-
- if (q->merge_bvec_fn(q, &bvm, prev) < prev->bv_len) {
- prev->bv_len -= len;
- return 0;
- }
- }
-
goto done;
}
}
@@ -768,31 +761,6 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page
bvec->bv_len = len;
bvec->bv_offset = offset;
- /*
- * if queue has other restrictions (eg varying max sector size
- * depending on offset), it can specify a merge_bvec_fn in the
- * queue to get further control
- */
- if (q->merge_bvec_fn) {
- struct bvec_merge_data bvm = {
- .bi_bdev = bio->bi_bdev,
- .bi_sector = bio->bi_iter.bi_sector,
- .bi_size = bio->bi_iter.bi_size,
- .bi_rw = bio->bi_rw,
- };
-
- /*
- * merge_bvec_fn() returns number of bytes it can accept
- * at this offset
- */
- if (q->merge_bvec_fn(q, &bvm, bvec) < bvec->bv_len) {
- bvec->bv_page = NULL;
- bvec->bv_len = 0;
- bvec->bv_offset = 0;
- return 0;
- }
- }
-
/* If we may be able to merge these biovecs, force a recount */
if (bio->bi_vcnt && (BIOVEC_PHYS_MERGEABLE(bvec-1, bvec)))
bio->bi_flags &= ~(1 << BIO_SEG_VALID);
@@ -803,28 +771,6 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page
bio->bi_iter.bi_size += len;
return len;
}
-
-/**
- * bio_add_pc_page - attempt to add page to bio
- * @q: the target queue
- * @bio: destination bio
- * @page: page to add
- * @len: vec entry length
- * @offset: vec entry offset
- *
- * Attempt to add a page to the bio_vec maplist. This can fail for a
- * number of reasons, such as the bio being full or target block device
- * limitations. The target block device must allow bio's up to PAGE_SIZE,
- * so it is always possible to add a single page to an empty bio.
- *
- * This should only be used by REQ_PC bios.
- */
-int bio_add_pc_page(struct request_queue *q, struct bio *bio, struct page *page,
- unsigned int len, unsigned int offset)
-{
- return __bio_add_page(q, bio, page, len, offset,
- queue_max_hw_sectors(q));
-}
EXPORT_SYMBOL(bio_add_pc_page);
/**
@@ -834,16 +780,47 @@ EXPORT_SYMBOL(bio_add_pc_page);
* @len: vec entry length
* @offset: vec entry offset
*
- * Attempt to add a page to the bio_vec maplist. This can fail for a
- * number of reasons, such as the bio being full or target block device
- * limitations. The target block device must allow bio's up to PAGE_SIZE,
- * so it is always possible to add a single page to an empty bio.
+ * Attempt to add a page to the bio_vec maplist. This will only fail if
+ * bio->bi_vcnt == bio->bi_max_vecs.
*/
-int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
- unsigned int offset)
+int bio_add_page(struct bio *bio, struct page *page,
+ unsigned int len, unsigned int offset)
{
- struct request_queue *q = bdev_get_queue(bio->bi_bdev);
- return __bio_add_page(q, bio, page, len, offset, queue_max_sectors(q));
+ struct bio_vec *bv;
+
+ /*
+ * cloned bio must not modify vec list
+ */
+ if (unlikely(bio_flagged(bio, BIO_CLONED)))
+ return 0;
+
+ /*
+ * For filesystems with a blocksize smaller than the pagesize
+ * we will often be called with the same page as last time and
+ * a consecutive offset. Optimize this special case.
+ */
+ if (bio->bi_vcnt > 0) {
+ bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
+
+ if (page == bv->bv_page &&
+ offset == bv->bv_offset + bv->bv_len) {
+ bv->bv_len += len;
+ goto done;
+ }
+ }
+
+ if (bio->bi_vcnt >= bio->bi_max_vecs)
+ return 0;
+
+ bv = &bio->bi_io_vec[bio->bi_vcnt];
+ bv->bv_page = page;
+ bv->bv_len = len;
+ bv->bv_offset = offset;
+
+ bio->bi_vcnt++;
+done:
+ bio->bi_iter.bi_size += len;
+ return len;
}
EXPORT_SYMBOL(bio_add_page);
--
1.8.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now
2013-11-25 22:30 [PATCH] Make generic_make_request() handle arbitrary size bios Kent Overstreet
` (2 preceding siblings ...)
2013-11-25 22:30 ` [PATCH 3/5] block: Gut bio_add_page() Kent Overstreet
@ 2013-11-25 22:30 ` Kent Overstreet
2013-11-27 12:54 ` Christoph Hellwig
2013-11-25 22:30 ` [PATCH 5/5] bcache: " Kent Overstreet
4 siblings, 1 reply; 12+ messages in thread
From: Kent Overstreet @ 2013-11-25 22:30 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, axboe; +Cc: Kent Overstreet
generic_make_request() will now do for us what the code in blk-lib.c was
doing manually, with the bio_batch stuff - we still need some looping in
case we're trying to discard/zeroout more than around a gigabyte, but
when we can submit that much at a time doing the submissions in parallel
really shouldn't matter.
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Cc: Jens Axboe <axboe@kernel.dk>
---
block/blk-lib.c | 175 ++++++++++----------------------------------------------
1 file changed, 30 insertions(+), 145 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2da76c9..368c36a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -9,23 +9,6 @@
#include "blk.h"
-struct bio_batch {
- atomic_t done;
- unsigned long flags;
- struct completion *wait;
-};
-
-static void bio_batch_end_io(struct bio *bio, int err)
-{
- struct bio_batch *bb = bio->bi_private;
-
- if (err && (err != -EOPNOTSUPP))
- clear_bit(BIO_UPTODATE, &bb->flags);
- if (atomic_dec_and_test(&bb->done))
- complete(bb->wait);
- bio_put(bio);
-}
-
/**
* blkdev_issue_discard - queue a discard
* @bdev: blockdev to issue discard for
@@ -40,15 +23,10 @@ static void bio_batch_end_io(struct bio *bio, int err)
int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned long flags)
{
- DECLARE_COMPLETION_ONSTACK(wait);
struct request_queue *q = bdev_get_queue(bdev);
int type = REQ_WRITE | REQ_DISCARD;
- unsigned int max_discard_sectors, granularity;
- int alignment;
- struct bio_batch bb;
struct bio *bio;
int ret = 0;
- struct blk_plug plug;
if (!q)
return -ENXIO;
@@ -56,78 +34,28 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
if (!blk_queue_discard(q))
return -EOPNOTSUPP;
- /* Zero-sector (unknown) and one-sector granularities are the same. */
- granularity = max(q->limits.discard_granularity >> 9, 1U);
- alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
-
- /*
- * Ensure that max_discard_sectors is of the proper
- * granularity, so that requests stay aligned after a split.
- */
- max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
- max_discard_sectors -= max_discard_sectors % granularity;
- if (unlikely(!max_discard_sectors)) {
- /* Avoid infinite loop below. Being cautious never hurts. */
- return -EOPNOTSUPP;
- }
-
if (flags & BLKDEV_DISCARD_SECURE) {
if (!blk_queue_secdiscard(q))
return -EOPNOTSUPP;
type |= REQ_SECURE;
}
- atomic_set(&bb.done, 1);
- bb.flags = 1 << BIO_UPTODATE;
- bb.wait = &wait;
-
- blk_start_plug(&plug);
while (nr_sects) {
- unsigned int req_sects;
- sector_t end_sect, tmp;
-
bio = bio_alloc(gfp_mask, 1);
- if (!bio) {
- ret = -ENOMEM;
- break;
- }
-
- req_sects = min_t(sector_t, nr_sects, max_discard_sectors);
-
- /*
- * If splitting a request, and the next starting sector would be
- * misaligned, stop the discard at the previous aligned sector.
- */
- end_sect = sector + req_sects;
- tmp = end_sect;
- if (req_sects < nr_sects &&
- sector_div(tmp, granularity) != alignment) {
- end_sect = end_sect - alignment;
- sector_div(end_sect, granularity);
- end_sect = end_sect * granularity + alignment;
- req_sects = end_sect - sector;
- }
+ if (!bio)
+ return -ENOMEM;
- bio->bi_iter.bi_sector = sector;
- bio->bi_end_io = bio_batch_end_io;
bio->bi_bdev = bdev;
- bio->bi_private = &bb;
+ bio->bi_iter.bi_sector = sector;
+ bio->bi_iter.bi_size = min_t(sector_t, nr_sects, 1 << 20) << 9;
- bio->bi_iter.bi_size = req_sects << 9;
- nr_sects -= req_sects;
- sector = end_sect;
+ sector += bio_sectors(bio);
+ nr_sects -= bio_sectors(bio);
- atomic_inc(&bb.done);
- submit_bio(type, bio);
+ ret = submit_bio_wait(type, bio);
+ if (ret)
+ break;
}
- blk_finish_plug(&plug);
-
- /* Wait for bios in-flight */
- if (!atomic_dec_and_test(&bb.done))
- wait_for_completion_io(&wait);
-
- if (!test_bit(BIO_UPTODATE, &bb.flags))
- ret = -EIO;
return ret;
}
@@ -148,61 +76,37 @@ int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask,
struct page *page)
{
- DECLARE_COMPLETION_ONSTACK(wait);
struct request_queue *q = bdev_get_queue(bdev);
- unsigned int max_write_same_sectors;
- struct bio_batch bb;
struct bio *bio;
int ret = 0;
if (!q)
return -ENXIO;
- max_write_same_sectors = q->limits.max_write_same_sectors;
-
- if (max_write_same_sectors == 0)
+ if (!q->limits.max_write_same_sectors)
return -EOPNOTSUPP;
- atomic_set(&bb.done, 1);
- bb.flags = 1 << BIO_UPTODATE;
- bb.wait = &wait;
-
while (nr_sects) {
bio = bio_alloc(gfp_mask, 1);
- if (!bio) {
- ret = -ENOMEM;
- break;
- }
+ if (!bio)
+ return -ENOMEM;
- bio->bi_iter.bi_sector = sector;
- bio->bi_end_io = bio_batch_end_io;
bio->bi_bdev = bdev;
- bio->bi_private = &bb;
+ bio->bi_iter.bi_sector = sector;
+ bio->bi_iter.bi_size = min_t(sector_t, nr_sects, 1 << 20) << 9;
bio->bi_vcnt = 1;
bio->bi_io_vec->bv_page = page;
bio->bi_io_vec->bv_offset = 0;
bio->bi_io_vec->bv_len = bdev_logical_block_size(bdev);
- if (nr_sects > max_write_same_sectors) {
- bio->bi_iter.bi_size = max_write_same_sectors << 9;
- nr_sects -= max_write_same_sectors;
- sector += max_write_same_sectors;
- } else {
- bio->bi_iter.bi_size = nr_sects << 9;
- nr_sects = 0;
- }
+ sector += bio_sectors(bio);
+ nr_sects -= bio_sectors(bio);
- atomic_inc(&bb.done);
- submit_bio(REQ_WRITE | REQ_WRITE_SAME, bio);
+ ret = submit_bio_wait(REQ_WRITE | REQ_WRITE_SAME, bio);
+ if (ret)
+ break;
}
- /* Wait for bios in-flight */
- if (!atomic_dec_and_test(&bb.done))
- wait_for_completion_io(&wait);
-
- if (!test_bit(BIO_UPTODATE, &bb.flags))
- ret = -ENOTSUPP;
-
return ret;
}
EXPORT_SYMBOL(blkdev_issue_write_same);
@@ -217,33 +121,22 @@ EXPORT_SYMBOL(blkdev_issue_write_same);
* Description:
* Generate and issue number of bios with zerofiled pages.
*/
-
int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
- sector_t nr_sects, gfp_t gfp_mask)
+ sector_t nr_sects, gfp_t gfp_mask)
{
- int ret;
+ int ret = 0;
struct bio *bio;
- struct bio_batch bb;
unsigned int sz;
- DECLARE_COMPLETION_ONSTACK(wait);
-
- atomic_set(&bb.done, 1);
- bb.flags = 1 << BIO_UPTODATE;
- bb.wait = &wait;
- ret = 0;
- while (nr_sects != 0) {
+ while (nr_sects) {
bio = bio_alloc(gfp_mask,
- min(nr_sects, (sector_t)BIO_MAX_PAGES));
- if (!bio) {
- ret = -ENOMEM;
- break;
- }
+ min(nr_sects / (PAGE_SIZE >> 9),
+ (sector_t)BIO_MAX_PAGES));
+ if (!bio)
+ return -ENOMEM;
bio->bi_iter.bi_sector = sector;
bio->bi_bdev = bdev;
- bio->bi_end_io = bio_batch_end_io;
- bio->bi_private = &bb;
while (nr_sects != 0) {
sz = min((sector_t) PAGE_SIZE >> 9 , nr_sects);
@@ -253,18 +146,11 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
if (ret < (sz << 9))
break;
}
- ret = 0;
- atomic_inc(&bb.done);
- submit_bio(WRITE, bio);
- }
-
- /* Wait for bios in-flight */
- if (!atomic_dec_and_test(&bb.done))
- wait_for_completion_io(&wait);
- if (!test_bit(BIO_UPTODATE, &bb.flags))
- /* One of bios in the batch was completed with error.*/
- ret = -EIO;
+ ret = submit_bio_wait(WRITE, bio);
+ if (ret)
+ break;
+ }
return ret;
}
@@ -279,7 +165,6 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
* Description:
* Generate and issue number of bios with zerofiled pages.
*/
-
int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask)
{
--
1.8.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now
2013-11-25 22:30 ` [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now Kent Overstreet
@ 2013-11-27 12:54 ` Christoph Hellwig
2013-11-27 21:34 ` Kent Overstreet
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2013-11-27 12:54 UTC (permalink / raw)
To: Kent Overstreet; +Cc: linux-kernel, linux-fsdevel, axboe
> int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, unsigned long flags)
> {
> - DECLARE_COMPLETION_ONSTACK(wait);
> struct request_queue *q = bdev_get_queue(bdev);
> int type = REQ_WRITE | REQ_DISCARD;
> - unsigned int max_discard_sectors, granularity;
> - int alignment;
> - struct bio_batch bb;
> struct bio *bio;
> int ret = 0;
> - struct blk_plug plug;
>
> if (!q)
> return -ENXIO;
> @@ -56,78 +34,28 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> if (!blk_queue_discard(q))
> return -EOPNOTSUPP;
>
> - /* Zero-sector (unknown) and one-sector granularities are the same. */
> - granularity = max(q->limits.discard_granularity >> 9, 1U);
> - alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
> -
> - /*
> - * Ensure that max_discard_sectors is of the proper
> - * granularity, so that requests stay aligned after a split.
> - */
> - max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> - max_discard_sectors -= max_discard_sectors % granularity;
Seems like this is also missing the code to trim down the request to
be properly aligned to the device limit. I don't think our low-level
drivers are ready to handle this, and even if they were it should be a
separate change.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now
2013-11-27 12:54 ` Christoph Hellwig
@ 2013-11-27 21:34 ` Kent Overstreet
0 siblings, 0 replies; 12+ messages in thread
From: Kent Overstreet @ 2013-11-27 21:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel, linux-fsdevel, axboe
On Wed, Nov 27, 2013 at 04:54:05AM -0800, Christoph Hellwig wrote:
> > int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> > sector_t nr_sects, gfp_t gfp_mask, unsigned long flags)
> > {
> > - DECLARE_COMPLETION_ONSTACK(wait);
> > struct request_queue *q = bdev_get_queue(bdev);
> > int type = REQ_WRITE | REQ_DISCARD;
> > - unsigned int max_discard_sectors, granularity;
> > - int alignment;
> > - struct bio_batch bb;
> > struct bio *bio;
> > int ret = 0;
> > - struct blk_plug plug;
> >
> > if (!q)
> > return -ENXIO;
> > @@ -56,78 +34,28 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> > if (!blk_queue_discard(q))
> > return -EOPNOTSUPP;
> >
> > - /* Zero-sector (unknown) and one-sector granularities are the same. */
> > - granularity = max(q->limits.discard_granularity >> 9, 1U);
> > - alignment = (bdev_discard_alignment(bdev) >> 9) % granularity;
> > -
> > - /*
> > - * Ensure that max_discard_sectors is of the proper
> > - * granularity, so that requests stay aligned after a split.
> > - */
> > - max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
> > - max_discard_sectors -= max_discard_sectors % granularity;
>
> Seems like this is also missing the code to trim down the request to
> be properly aligned to the device limit. I don't think our low-level
> drivers are ready to handle this, and even if they were it should be a
> separate change.
The previous patch added that - that was blk_bio_discard_split(), which you
commented on...
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] bcache: generic_make_request() handles large bios now
2013-11-25 22:30 [PATCH] Make generic_make_request() handle arbitrary size bios Kent Overstreet
` (3 preceding siblings ...)
2013-11-25 22:30 ` [PATCH 4/5] blk-lib.c: generic_make_request() handles large bios now Kent Overstreet
@ 2013-11-25 22:30 ` Kent Overstreet
4 siblings, 0 replies; 12+ messages in thread
From: Kent Overstreet @ 2013-11-25 22:30 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, axboe; +Cc: Kent Overstreet
So we get to delete our hacky workaround.
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
---
drivers/md/bcache/bcache.h | 18 --------
drivers/md/bcache/io.c | 100 +-----------------------------------------
drivers/md/bcache/journal.c | 4 +-
drivers/md/bcache/request.c | 16 +++----
drivers/md/bcache/super.c | 32 +-------------
drivers/md/bcache/util.h | 5 ++-
drivers/md/bcache/writeback.c | 4 +-
7 files changed, 18 insertions(+), 161 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 964353c..8f65331 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -241,19 +241,6 @@ struct keybuf {
DECLARE_ARRAY_ALLOCATOR(struct keybuf_key, freelist, KEYBUF_NR);
};
-struct bio_split_pool {
- struct bio_set *bio_split;
- mempool_t *bio_split_hook;
-};
-
-struct bio_split_hook {
- struct closure cl;
- struct bio_split_pool *p;
- struct bio *bio;
- bio_end_io_t *bi_end_io;
- void *bi_private;
-};
-
struct bcache_device {
struct closure cl;
@@ -286,8 +273,6 @@ struct bcache_device {
int (*cache_miss)(struct btree *, struct search *,
struct bio *, unsigned);
int (*ioctl) (struct bcache_device *, fmode_t, unsigned, unsigned long);
-
- struct bio_split_pool bio_split_hook;
};
struct io {
@@ -465,8 +450,6 @@ struct cache {
atomic_long_t meta_sectors_written;
atomic_long_t btree_sectors_written;
atomic_long_t sectors_written;
-
- struct bio_split_pool bio_split_hook;
};
struct gc_stat {
@@ -901,7 +884,6 @@ void bch_bbio_endio(struct cache_set *, struct bio *, int, const char *);
void bch_bbio_free(struct bio *, struct cache_set *);
struct bio *bch_bbio_alloc(struct cache_set *);
-void bch_generic_make_request(struct bio *, struct bio_split_pool *);
void __bch_submit_bbio(struct bio *, struct cache_set *);
void bch_submit_bbio(struct bio *, struct cache_set *, struct bkey *, unsigned);
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index fa028fa..86a0bb8 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -11,104 +11,6 @@
#include <linux/blkdev.h>
-static unsigned bch_bio_max_sectors(struct bio *bio)
-{
- struct request_queue *q = bdev_get_queue(bio->bi_bdev);
- struct bio_vec bv;
- struct bvec_iter iter;
- unsigned ret = 0, seg = 0;
-
- if (bio->bi_rw & REQ_DISCARD)
- return min(bio_sectors(bio), q->limits.max_discard_sectors);
-
- bio_for_each_segment(bv, bio, iter) {
- struct bvec_merge_data bvm = {
- .bi_bdev = bio->bi_bdev,
- .bi_sector = bio->bi_iter.bi_sector,
- .bi_size = ret << 9,
- .bi_rw = bio->bi_rw,
- };
-
- if (seg == min_t(unsigned, BIO_MAX_PAGES,
- queue_max_segments(q)))
- break;
-
- if (q->merge_bvec_fn &&
- q->merge_bvec_fn(q, &bvm, &bv) < (int) bv.bv_len)
- break;
-
- seg++;
- ret += bv.bv_len >> 9;
- }
-
- ret = min(ret, queue_max_sectors(q));
-
- WARN_ON(!ret);
- ret = max_t(int, ret, bio_iovec(bio).bv_len >> 9);
-
- return ret;
-}
-
-static void bch_bio_submit_split_done(struct closure *cl)
-{
- struct bio_split_hook *s = container_of(cl, struct bio_split_hook, cl);
-
- s->bio->bi_end_io = s->bi_end_io;
- s->bio->bi_private = s->bi_private;
- bio_endio_nodec(s->bio, 0);
-
- closure_debug_destroy(&s->cl);
- mempool_free(s, s->p->bio_split_hook);
-}
-
-static void bch_bio_submit_split_endio(struct bio *bio, int error)
-{
- struct closure *cl = bio->bi_private;
- struct bio_split_hook *s = container_of(cl, struct bio_split_hook, cl);
-
- if (error)
- clear_bit(BIO_UPTODATE, &s->bio->bi_flags);
-
- bio_put(bio);
- closure_put(cl);
-}
-
-void bch_generic_make_request(struct bio *bio, struct bio_split_pool *p)
-{
- struct bio_split_hook *s;
- struct bio *n;
-
- if (!bio_has_data(bio) && !(bio->bi_rw & REQ_DISCARD))
- goto submit;
-
- if (bio_sectors(bio) <= bch_bio_max_sectors(bio))
- goto submit;
-
- s = mempool_alloc(p->bio_split_hook, GFP_NOIO);
- closure_init(&s->cl, NULL);
-
- s->bio = bio;
- s->p = p;
- s->bi_end_io = bio->bi_end_io;
- s->bi_private = bio->bi_private;
- bio_get(bio);
-
- do {
- n = bio_next_split(bio, bch_bio_max_sectors(bio),
- GFP_NOIO, s->p->bio_split);
-
- n->bi_end_io = bch_bio_submit_split_endio;
- n->bi_private = &s->cl;
-
- closure_get(&s->cl);
- generic_make_request(n);
- } while (n != bio);
-
- continue_at(&s->cl, bch_bio_submit_split_done, NULL);
-submit:
- generic_make_request(bio);
-}
-
/* Bios with headers */
void bch_bbio_free(struct bio *bio, struct cache_set *c)
@@ -138,7 +40,7 @@ void __bch_submit_bbio(struct bio *bio, struct cache_set *c)
bio->bi_bdev = PTR_CACHE(c, &b->key, 0)->bdev;
b->submit_time_us = local_clock_us();
- closure_bio_submit(bio, bio->bi_private, PTR_CACHE(c, &b->key, 0));
+ closure_bio_submit(bio, bio->bi_private);
}
void bch_submit_bbio(struct bio *bio, struct cache_set *c,
diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 7eafdf0..15aeebe 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -60,7 +60,7 @@ reread: left = ca->sb.bucket_size - offset;
bio->bi_private = &cl;
bch_bio_map(bio, data);
- closure_bio_submit(bio, &cl, ca);
+ closure_bio_submit(bio, &cl);
closure_sync(&cl);
/* This function could be simpler now since we no longer write
@@ -632,7 +632,7 @@ static void journal_write_unlocked(struct closure *cl)
spin_unlock(&c->journal.lock);
while ((bio = bio_list_pop(&list)))
- closure_bio_submit(bio, cl, c->cache[0]);
+ closure_bio_submit(bio, cl);
continue_at(cl, journal_write_done, NULL);
}
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 5878cdb..bf02d51 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -848,7 +848,7 @@ static void cached_dev_read_error(struct closure *cl)
/* XXX: invalidate cache */
- closure_bio_submit(bio, cl, s->d);
+ closure_bio_submit(bio, cl);
}
continue_at(cl, cached_dev_cache_miss_done, NULL);
@@ -971,7 +971,7 @@ static int cached_dev_cache_miss(struct btree *b, struct search *s,
s->cache_miss = miss;
s->iop.bio = cache_bio;
bio_get(cache_bio);
- closure_bio_submit(cache_bio, &s->cl, s->d);
+ closure_bio_submit(cache_bio, &s->cl);
return ret;
out_put:
@@ -979,7 +979,7 @@ out_put:
out_submit:
miss->bi_end_io = request_endio;
miss->bi_private = &s->cl;
- closure_bio_submit(miss, &s->cl, s->d);
+ closure_bio_submit(miss, &s->cl);
return ret;
}
@@ -1044,7 +1044,7 @@ static void cached_dev_write(struct cached_dev *dc, struct search *s)
if (!(bio->bi_rw & REQ_DISCARD) ||
blk_queue_discard(bdev_get_queue(dc->bdev)))
- closure_bio_submit(bio, cl, s->d);
+ closure_bio_submit(bio, cl);
} else if (s->iop.writeback) {
bch_writeback_add(dc);
s->iop.bio = bio;
@@ -1059,12 +1059,12 @@ static void cached_dev_write(struct cached_dev *dc, struct search *s)
flush->bi_end_io = request_endio;
flush->bi_private = cl;
- closure_bio_submit(flush, cl, s->d);
+ closure_bio_submit(flush, cl);
}
} else {
s->iop.bio = bio_clone_fast(bio, GFP_NOIO, dc->disk.bio_split);
- closure_bio_submit(bio, cl, s->d);
+ closure_bio_submit(bio, cl);
}
closure_call(&s->iop.cl, bch_data_insert, NULL, cl);
@@ -1080,7 +1080,7 @@ static void cached_dev_nodata(struct closure *cl)
bch_journal_meta(s->iop.c, cl);
/* If it's a flush, we send the flush to the backing device too */
- closure_bio_submit(bio, cl, s->d);
+ closure_bio_submit(bio, cl);
continue_at(cl, cached_dev_bio_complete, NULL);
}
@@ -1127,7 +1127,7 @@ static void cached_dev_make_request(struct request_queue *q, struct bio *bio)
!blk_queue_discard(bdev_get_queue(dc->bdev)))
bio_endio(bio, 0);
else
- bch_generic_make_request(bio, &d->bio_split_hook);
+ generic_make_request(bio);
}
}
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 60fb604..a596f39 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -58,29 +58,6 @@ struct workqueue_struct *bcache_wq;
#define BTREE_MAX_PAGES (256 * 1024 / PAGE_SIZE)
-static void bio_split_pool_free(struct bio_split_pool *p)
-{
- if (p->bio_split_hook)
- mempool_destroy(p->bio_split_hook);
-
- if (p->bio_split)
- bioset_free(p->bio_split);
-}
-
-static int bio_split_pool_init(struct bio_split_pool *p)
-{
- p->bio_split = bioset_create(4, 0);
- if (!p->bio_split)
- return -ENOMEM;
-
- p->bio_split_hook = mempool_create_kmalloc_pool(4,
- sizeof(struct bio_split_hook));
- if (!p->bio_split_hook)
- return -ENOMEM;
-
- return 0;
-}
-
/* Superblock */
static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
@@ -512,7 +489,7 @@ static void prio_io(struct cache *ca, uint64_t bucket, unsigned long rw)
bio->bi_private = ca;
bch_bio_map(bio, ca->disk_buckets);
- closure_bio_submit(bio, &ca->prio, ca);
+ closure_bio_submit(bio, &ca->prio);
closure_sync(cl);
}
@@ -738,7 +715,6 @@ static void bcache_device_free(struct bcache_device *d)
put_disk(d->disk);
}
- bio_split_pool_free(&d->bio_split_hook);
if (d->bio_split)
bioset_free(d->bio_split);
if (is_vmalloc_addr(d->full_dirty_stripes))
@@ -791,7 +767,6 @@ static int bcache_device_init(struct bcache_device *d, unsigned block_size,
return minor;
if (!(d->bio_split = bioset_create(4, offsetof(struct bbio, bio))) ||
- bio_split_pool_init(&d->bio_split_hook) ||
!(d->disk = alloc_disk(1))) {
ida_simple_remove(&bcache_minor, minor);
return -ENOMEM;
@@ -1747,8 +1722,6 @@ void bch_cache_release(struct kobject *kobj)
if (ca->set)
ca->set->cache[ca->sb.nr_this_dev] = NULL;
- bio_split_pool_free(&ca->bio_split_hook);
-
free_pages((unsigned long) ca->disk_buckets, ilog2(bucket_pages(ca)));
kfree(ca->prio_buckets);
vfree(ca->buckets);
@@ -1793,8 +1766,7 @@ static int cache_alloc(struct cache_sb *sb, struct cache *ca)
ca->sb.nbuckets)) ||
!(ca->prio_buckets = kzalloc(sizeof(uint64_t) * prio_buckets(ca) *
2, GFP_KERNEL)) ||
- !(ca->disk_buckets = alloc_bucket_pages(GFP_KERNEL, ca)) ||
- bio_split_pool_init(&ca->bio_split_hook))
+ !(ca->disk_buckets = alloc_bucket_pages(GFP_KERNEL, ca)))
return -ENOMEM;
ca->prio_last_buckets = ca->prio_buckets + prio_buckets(ca);
diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
index 362c4b3..4cafce4 100644
--- a/drivers/md/bcache/util.h
+++ b/drivers/md/bcache/util.h
@@ -3,6 +3,7 @@
#define _BCACHE_UTIL_H
#include <linux/errno.h>
+#include <linux/blkdev.h>
#include <linux/kernel.h>
#include <linux/llist.h>
#include <linux/ratelimit.h>
@@ -568,10 +569,10 @@ static inline sector_t bdev_sectors(struct block_device *bdev)
return bdev->bd_inode->i_size >> 9;
}
-#define closure_bio_submit(bio, cl, dev) \
+#define closure_bio_submit(bio, cl) \
do { \
closure_get(cl); \
- bch_generic_make_request(bio, &(dev)->bio_split_hook); \
+ generic_make_request(bio); \
} while (0)
uint64_t bch_crc64_update(uint64_t, const void *, size_t);
diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
index 04657e9..b3ce66e 100644
--- a/drivers/md/bcache/writeback.c
+++ b/drivers/md/bcache/writeback.c
@@ -190,7 +190,7 @@ static void write_dirty(struct closure *cl)
io->bio.bi_bdev = io->dc->bdev;
io->bio.bi_end_io = dirty_endio;
- closure_bio_submit(&io->bio, cl, &io->dc->disk);
+ closure_bio_submit(&io->bio, cl);
continue_at(cl, write_dirty_finish, system_wq);
}
@@ -210,7 +210,7 @@ static void read_dirty_submit(struct closure *cl)
{
struct dirty_io *io = container_of(cl, struct dirty_io, cl);
- closure_bio_submit(&io->bio, cl, &io->dc->disk);
+ closure_bio_submit(&io->bio, cl);
continue_at(cl, write_dirty, system_wq);
}
--
1.8.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread