* [PATCH 1/3] block: don't use bio->bi_vcnt to figure out segment number
2018-07-31 10:49 [PATCH 0/3] block: fix mismatch of figuring out physical segment number Ming Lei
@ 2018-07-31 10:49 ` Ming Lei
2018-08-01 14:52 ` Christoph Hellwig
2018-07-31 10:49 ` [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE Ming Lei
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Ming Lei @ 2018-07-31 10:49 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Christoph Hellwig, Mike Snitzer, Kent Overstreet
It is wrong to use bio->bi_vcnt to figure out how many segments
there are in the bio even though CLONED flag isn't set on this bio,
because this bio may be splitted or advanced.
So always use bio_segments() in blk_recount_segments(), and it shouldn't
cause any performance loss now because the physical segment number is figured
out in blk_queue_split() and BIO_SEG_VALID is set meantime since
bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting").
Fixes: 7f60dcaaf91 ("block: blk-merge: fix blk_recount_segments()")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-merge.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index aaec38cc37b8..4a16d4f929da 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -308,13 +308,7 @@ void blk_recalc_rq_segments(struct request *rq)
void blk_recount_segments(struct request_queue *q, struct bio *bio)
{
- unsigned short seg_cnt;
-
- /* estimate segment number by bi_vcnt for non-cloned bio */
- if (bio_flagged(bio, BIO_CLONED))
- seg_cnt = bio_segments(bio);
- else
- seg_cnt = bio->bi_vcnt;
+ unsigned short seg_cnt = bio_segments(bio);
if (test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags) &&
(seg_cnt < queue_max_segments(q)))
--
2.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] block: don't use bio->bi_vcnt to figure out segment number
2018-07-31 10:49 ` [PATCH 1/3] block: don't use bio->bi_vcnt to figure out " Ming Lei
@ 2018-08-01 14:52 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2018-08-01 14:52 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Mike Snitzer,
Kent Overstreet
On Tue, Jul 31, 2018 at 06:49:12PM +0800, Ming Lei wrote:
> It is wrong to use bio->bi_vcnt to figure out how many segments
> there are in the bio even though CLONED flag isn't set on this bio,
> because this bio may be splitted or advanced.
>
> So always use bio_segments() in blk_recount_segments(), and it shouldn't
> cause any performance loss now because the physical segment number is figured
> out in blk_queue_split() and BIO_SEG_VALID is set meantime since
> bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting").
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE
2018-07-31 10:49 [PATCH 0/3] block: fix mismatch of figuring out physical segment number Ming Lei
2018-07-31 10:49 ` [PATCH 1/3] block: don't use bio->bi_vcnt to figure out " Ming Lei
@ 2018-07-31 10:49 ` Ming Lei
2018-08-01 14:54 ` Christoph Hellwig
2018-07-31 10:49 ` [PATCH 3/3] block: kill BLK_MQ_F_SG_MERGE Ming Lei
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Ming Lei @ 2018-07-31 10:49 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Christoph Hellwig, Mike Snitzer, Kent Overstreet
Since bdced438acd83ad83a6c ("block: setup bi_phys_segments after splitting"),
physical segment number is mainly figured out in blk_queue_split() for
fast path, and the flag of BIO_SEG_VALID is set there too.
Now only blk_recount_segments() and blk_recalc_rq_segments() use this
flag.
Basically blk_recount_segments() is bypassed in fast path given BIO_SEG_VALID
is set in blk_queue_split().
For another user of blk_recalc_rq_segments():
- run in partial completion branch of blk_update_request, which is an unusual case
- run in blk_cloned_rq_check_limits(), still not a big problem if the flag is killed
since dm-rq is the only user.
Also multipage bvec will come soon, and each bvec may become a one segment
at that time, so QUEUE_FLAG_NO_SG_MERGE doesn't make sense any more.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-merge.c | 31 ++++++-------------------------
block/blk-mq-debugfs.c | 1 -
block/blk-mq.c | 3 ---
drivers/md/dm-table.c | 13 -------------
include/linux/blkdev.h | 1 -
5 files changed, 6 insertions(+), 43 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 4a16d4f929da..9ee650aeda58 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -229,8 +229,7 @@ void blk_queue_split(struct request_queue *q, struct bio **bio)
EXPORT_SYMBOL(blk_queue_split);
static unsigned int __blk_recalc_rq_segments(struct request_queue *q,
- struct bio *bio,
- bool no_sg_merge)
+ struct bio *bio)
{
struct bio_vec bv, bvprv = { NULL };
int cluster, prev = 0;
@@ -256,13 +255,6 @@ 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) {
- /*
- * If SG merging is disabled, each bio vector is
- * a segment
- */
- if (no_sg_merge)
- goto new_segment;
-
if (prev && cluster) {
if (seg_size + bv.bv_len
> queue_max_segment_size(q))
@@ -299,27 +291,16 @@ static unsigned int __blk_recalc_rq_segments(struct request_queue *q,
void blk_recalc_rq_segments(struct request *rq)
{
- bool no_sg_merge = !!test_bit(QUEUE_FLAG_NO_SG_MERGE,
- &rq->q->queue_flags);
-
- rq->nr_phys_segments = __blk_recalc_rq_segments(rq->q, rq->bio,
- no_sg_merge);
+ rq->nr_phys_segments = __blk_recalc_rq_segments(rq->q, rq->bio);
}
void blk_recount_segments(struct request_queue *q, struct bio *bio)
{
- unsigned short seg_cnt = bio_segments(bio);
-
- if (test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags) &&
- (seg_cnt < queue_max_segments(q)))
- bio->bi_phys_segments = seg_cnt;
- else {
- struct bio *nxt = bio->bi_next;
+ struct bio *nxt = bio->bi_next;
- bio->bi_next = NULL;
- bio->bi_phys_segments = __blk_recalc_rq_segments(q, bio, false);
- bio->bi_next = nxt;
- }
+ bio->bi_next = NULL;
+ bio->bi_phys_segments = __blk_recalc_rq_segments(q, bio);
+ bio->bi_next = nxt;
bio_set_flag(bio, BIO_SEG_VALID);
}
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index cb1e6cf7ac48..e590f9495d25 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -121,7 +121,6 @@ static const char *const blk_queue_flag_name[] = {
QUEUE_FLAG_NAME(SAME_FORCE),
QUEUE_FLAG_NAME(DEAD),
QUEUE_FLAG_NAME(INIT_DONE),
- QUEUE_FLAG_NAME(NO_SG_MERGE),
QUEUE_FLAG_NAME(POLL),
QUEUE_FLAG_NAME(WC),
QUEUE_FLAG_NAME(FUA),
diff --git a/block/blk-mq.c b/block/blk-mq.c
index e13bdc2707ce..dc90be8206d2 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2606,9 +2606,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
- if (!(set->flags & BLK_MQ_F_SG_MERGE))
- queue_flag_set_unlocked(QUEUE_FLAG_NO_SG_MERGE, q);
-
q->sg_reserved_size = INT_MAX;
INIT_DELAYED_WORK(&q->requeue_work, blk_mq_requeue_work);
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 3d0e2c198f06..5ea54c5bd508 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1718,14 +1718,6 @@ static int device_is_not_random(struct dm_target *ti, struct dm_dev *dev,
return q && !blk_queue_add_random(q);
}
-static int queue_supports_sg_merge(struct dm_target *ti, struct dm_dev *dev,
- sector_t start, sector_t len, void *data)
-{
- struct request_queue *q = bdev_get_queue(dev->bdev);
-
- return q && !test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags);
-}
-
static bool dm_table_all_devices_attribute(struct dm_table *t,
iterate_devices_callout_fn func)
{
@@ -1922,11 +1914,6 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
if (!dm_table_supports_write_zeroes(t))
q->limits.max_write_zeroes_sectors = 0;
- if (dm_table_all_devices_attribute(t, queue_supports_sg_merge))
- blk_queue_flag_clear(QUEUE_FLAG_NO_SG_MERGE, q);
- else
- blk_queue_flag_set(QUEUE_FLAG_NO_SG_MERGE, q);
-
dm_table_verify_integrity(t);
/*
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 050d599f5ea9..0697d30bb625 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -689,7 +689,6 @@ struct request_queue {
#define QUEUE_FLAG_SAME_FORCE 15 /* force complete on same CPU */
#define QUEUE_FLAG_DEAD 16 /* queue tear-down finished */
#define QUEUE_FLAG_INIT_DONE 17 /* queue is initialized */
-#define QUEUE_FLAG_NO_SG_MERGE 18 /* don't attempt to merge SG segments*/
#define QUEUE_FLAG_POLL 19 /* IO polling enabled if set */
#define QUEUE_FLAG_WC 20 /* Write back caching */
#define QUEUE_FLAG_FUA 21 /* device supports FUA writes */
--
2.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE
2018-07-31 10:49 ` [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE Ming Lei
@ 2018-08-01 14:54 ` Christoph Hellwig
2018-08-01 23:11 ` Ming Lei
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2018-08-01 14:54 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Mike Snitzer,
Kent Overstreet
We still hit __blk_recalc_rq_segments at least once per submission,
don't we? Either rather wait with this until we have multi-page bvecs.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE
2018-08-01 14:54 ` Christoph Hellwig
@ 2018-08-01 23:11 ` Ming Lei
0 siblings, 0 replies; 9+ messages in thread
From: Ming Lei @ 2018-08-01 23:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, Mike Snitzer, Kent Overstreet
On Wed, Aug 01, 2018 at 04:54:02PM +0200, Christoph Hellwig wrote:
> We still hit __blk_recalc_rq_segments at least once per submission,
> don't we? Either rather wait with this until we have multi-page bvecs.
No.
Please see blk_queue_split(), where the physical segment number is
always updated, and the flag of BIO_SEG_VALID is set meantime.
And for callers of blk_recount_segments(), it will check the VALID flag
first, and blk_recount_segments() is only run when this flag isn't set.
blk_recount_segments
bio_phys_segments
ll_new_hw_segment
ll_back_merge_fn
ll_front_merge_fn
blk_rq_bio_prep
blk_rq_append_bio
blk_init_request_from_bio
bio_add_pc_page
ll_back_merge_fn
ll_front_merge_fn
Thanks,
Ming
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] block: kill BLK_MQ_F_SG_MERGE
2018-07-31 10:49 [PATCH 0/3] block: fix mismatch of figuring out physical segment number Ming Lei
2018-07-31 10:49 ` [PATCH 1/3] block: don't use bio->bi_vcnt to figure out " Ming Lei
2018-07-31 10:49 ` [PATCH 2/3] block: kill QUEUE_FLAG_NO_SG_MERGE Ming Lei
@ 2018-07-31 10:49 ` Ming Lei
2018-07-31 15:32 ` [PATCH 0/3] block: fix mismatch of figuring out physical segment number Mike Snitzer
2018-08-06 2:37 ` Ming Lei
4 siblings, 0 replies; 9+ messages in thread
From: Ming Lei @ 2018-07-31 10:49 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Christoph Hellwig, Mike Snitzer, Kent Overstreet
QUEUE_FLAG_NO_SG_MERGE has been killed, so kill BLK_MQ_F_SG_MERGE too.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-debugfs.c | 1 -
drivers/block/loop.c | 2 +-
drivers/block/nbd.c | 2 +-
drivers/block/rbd.c | 2 +-
drivers/block/skd_main.c | 1 -
drivers/block/xen-blkfront.c | 2 +-
drivers/md/dm-rq.c | 2 +-
drivers/mmc/core/queue.c | 3 +--
drivers/scsi/scsi_lib.c | 2 +-
include/linux/blk-mq.h | 1 -
10 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index e590f9495d25..cf1b83b4b737 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -243,7 +243,6 @@ static const char *const alloc_policy_name[] = {
static const char *const hctx_flag_name[] = {
HCTX_FLAG_NAME(SHOULD_MERGE),
HCTX_FLAG_NAME(TAG_SHARED),
- HCTX_FLAG_NAME(SG_MERGE),
HCTX_FLAG_NAME(BLOCKING),
HCTX_FLAG_NAME(NO_SCHED),
};
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ea9debf59b22..50769942f38e 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1846,7 +1846,7 @@ static int loop_add(struct loop_device **l, int i)
lo->tag_set.queue_depth = 128;
lo->tag_set.numa_node = NUMA_NO_NODE;
lo->tag_set.cmd_size = sizeof(struct loop_cmd);
- lo->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+ lo->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
lo->tag_set.driver_data = lo;
err = blk_mq_alloc_tag_set(&lo->tag_set);
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 74a05561b620..71a66894b941 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1506,7 +1506,7 @@ static int nbd_dev_add(int index)
nbd->tag_set.numa_node = NUMA_NO_NODE;
nbd->tag_set.cmd_size = sizeof(struct nbd_cmd);
nbd->tag_set.flags = BLK_MQ_F_SHOULD_MERGE |
- BLK_MQ_F_SG_MERGE | BLK_MQ_F_BLOCKING;
+ BLK_MQ_F_BLOCKING;
nbd->tag_set.driver_data = nbd;
err = blk_mq_alloc_tag_set(&nbd->tag_set);
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index fa0729c1e776..0bd0b2e7db75 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3964,7 +3964,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
rbd_dev->tag_set.ops = &rbd_mq_ops;
rbd_dev->tag_set.queue_depth = rbd_dev->opts->queue_depth;
rbd_dev->tag_set.numa_node = NUMA_NO_NODE;
- rbd_dev->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+ rbd_dev->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
rbd_dev->tag_set.nr_hw_queues = 1;
rbd_dev->tag_set.cmd_size = sizeof(struct work_struct);
diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 87b9e7fbf062..91ba954fcbd3 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -2834,7 +2834,6 @@ static int skd_cons_disk(struct skd_device *skdev)
skdev->sgs_per_request * sizeof(struct scatterlist);
skdev->tag_set.numa_node = NUMA_NO_NODE;
skdev->tag_set.flags = BLK_MQ_F_SHOULD_MERGE |
- BLK_MQ_F_SG_MERGE |
BLK_ALLOC_POLICY_TO_MQ_FLAG(BLK_TAG_ALLOC_FIFO);
skdev->tag_set.driver_data = skdev;
rc = blk_mq_alloc_tag_set(&skdev->tag_set);
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 94300dbe358b..e0723c63fe6a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -973,7 +973,7 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
} else
info->tag_set.queue_depth = BLK_RING_SIZE(info);
info->tag_set.numa_node = NUMA_NO_NODE;
- info->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+ info->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
info->tag_set.cmd_size = sizeof(struct blkif_req);
info->tag_set.driver_data = info;
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 6e547b8dd298..5d45adb81d0e 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -802,7 +802,7 @@ int dm_mq_init_request_queue(struct mapped_device *md, struct dm_table *t)
md->tag_set->ops = &dm_mq_ops;
md->tag_set->queue_depth = dm_get_blk_mq_queue_depth();
md->tag_set->numa_node = md->numa_node_id;
- md->tag_set->flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+ md->tag_set->flags = BLK_MQ_F_SHOULD_MERGE;
md->tag_set->nr_hw_queues = dm_get_blk_mq_nr_hw_queues();
md->tag_set->driver_data = md;
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 648eb6743ed5..b38ef099e19f 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -385,8 +385,7 @@ static int mmc_mq_init_queue(struct mmc_queue *mq, int q_depth,
mq->tag_set.ops = mq_ops;
mq->tag_set.queue_depth = q_depth;
mq->tag_set.numa_node = NUMA_NO_NODE;
- mq->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE |
- BLK_MQ_F_BLOCKING;
+ mq->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_BLOCKING;
mq->tag_set.nr_hw_queues = 1;
mq->tag_set.cmd_size = sizeof(struct mmc_queue_req);
mq->tag_set.driver_data = mq;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 41e9ac9fc138..0759ead2683d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2306,7 +2306,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
shost->tag_set.queue_depth = shost->can_queue;
shost->tag_set.cmd_size = cmd_size;
shost->tag_set.numa_node = NUMA_NO_NODE;
- shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+ shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
shost->tag_set.flags |=
BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy);
shost->tag_set.driver_data = shost;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index d710e92874cc..98f3b239c561 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -179,7 +179,6 @@ struct blk_mq_ops {
enum {
BLK_MQ_F_SHOULD_MERGE = 1 << 0,
BLK_MQ_F_TAG_SHARED = 1 << 1,
- BLK_MQ_F_SG_MERGE = 1 << 2,
BLK_MQ_F_BLOCKING = 1 << 5,
BLK_MQ_F_NO_SCHED = 1 << 6,
BLK_MQ_F_ALLOC_POLICY_START_BIT = 8,
--
2.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] block: fix mismatch of figuring out physical segment number
2018-07-31 10:49 [PATCH 0/3] block: fix mismatch of figuring out physical segment number Ming Lei
` (2 preceding siblings ...)
2018-07-31 10:49 ` [PATCH 3/3] block: kill BLK_MQ_F_SG_MERGE Ming Lei
@ 2018-07-31 15:32 ` Mike Snitzer
2018-08-06 2:37 ` Ming Lei
4 siblings, 0 replies; 9+ messages in thread
From: Mike Snitzer @ 2018-07-31 15:32 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Christoph Hellwig, Kent Overstreet
On Tue, Jul 31 2018 at 6:49am -0400,
Ming Lei <ming.lei@redhat.com> wrote:
> Hi,
>
> This patchset fixes one issue related with physical segment computation,
> which is found by Mike. In case of dm-rq, the warning of 'blk_cloned_rq_check_limits:
> over max segments limit' can be triggered easily.
>
> Follows the cause:
>
> 1) in IO fast path(blk_queue_split()), we always figure out physical segment number
> no matter the flag of QUEUE_FLAG_NO_SG_MERGE is set or not.
>
> 2) only blk_recount_segments() and blk_recalc_rq_segments() uses the flag of
> QUEUE_FLAG_NO_SG_MERGE, but the two are only called in some unusual
> cases, such as request clone in dm-rq.
>
> 3) the above two computation don't match, and cause the warning of
> "blk_cloned_rq_check_limits: over max segments limit".
>
> This patchset fixes this issue by killing the queue flag since it is basically
> bypassed since v4.4, and no one complains that at all. Also multipage bvec will
> come soon, and it doesn't make sense to keep QUEUE_FLAG_NO_SG_MERGE any more.
Thanks Ming, for the series:
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] block: fix mismatch of figuring out physical segment number
2018-07-31 10:49 [PATCH 0/3] block: fix mismatch of figuring out physical segment number Ming Lei
` (3 preceding siblings ...)
2018-07-31 15:32 ` [PATCH 0/3] block: fix mismatch of figuring out physical segment number Mike Snitzer
@ 2018-08-06 2:37 ` Ming Lei
4 siblings, 0 replies; 9+ messages in thread
From: Ming Lei @ 2018-08-06 2:37 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Mike Snitzer,
Kent Overstreet
On Tue, Jul 31, 2018 at 6:49 PM, Ming Lei <ming.lei@redhat.com> wrote:
> Hi,
>
> This patchset fixes one issue related with physical segment computation,
> which is found by Mike. In case of dm-rq, the warning of 'blk_cloned_rq_check_limits:
> over max segments limit' can be triggered easily.
>
> Follows the cause:
>
> 1) in IO fast path(blk_queue_split()), we always figure out physical segment number
> no matter the flag of QUEUE_FLAG_NO_SG_MERGE is set or not.
>
> 2) only blk_recount_segments() and blk_recalc_rq_segments() uses the flag of
> QUEUE_FLAG_NO_SG_MERGE, but the two are only called in some unusual
> cases, such as request clone in dm-rq.
>
> 3) the above two computation don't match, and cause the warning of
> "blk_cloned_rq_check_limits: over max segments limit".
>
> This patchset fixes this issue by killing the queue flag since it is basically
> bypassed since v4.4, and no one complains that at all. Also multipage bvec will
> come soon, and it doesn't make sense to keep QUEUE_FLAG_NO_SG_MERGE any more.
>
>
> Ming Lei (3):
> block: don't use bio->bi_vcnt to figure out segment number
> block: kill QUEUE_FLAG_NO_SG_MERGE
> block: kill BLK_MQ_F_SG_MERGE
>
> block/blk-merge.c | 39 +++++++--------------------------------
> block/blk-mq-debugfs.c | 2 --
> block/blk-mq.c | 3 ---
> drivers/block/loop.c | 2 +-
> drivers/block/nbd.c | 2 +-
> drivers/block/rbd.c | 2 +-
> drivers/block/skd_main.c | 1 -
> drivers/block/xen-blkfront.c | 2 +-
> drivers/md/dm-rq.c | 2 +-
> drivers/md/dm-table.c | 13 -------------
> drivers/mmc/core/queue.c | 3 +--
> drivers/scsi/scsi_lib.c | 2 +-
> include/linux/blk-mq.h | 1 -
> include/linux/blkdev.h | 1 -
> 14 files changed, 14 insertions(+), 61 deletions(-)
>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Kent Overstreet <kent.overstreet@gmail.com>
Hello Guys,
Ping...
Thanks,
Ming Lei
^ permalink raw reply [flat|nested] 9+ messages in thread