* [PATCHv2 0/2] block: Discard merging fixes @ 2018-02-01 20:31 ` Keith Busch 0 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) To: Linux Block, Linux NVMe, Christoph Hellwig, Jens Axboe; +Cc: Keith Busch This series fixes some incorrect accounting from merging discard requests. The first patch fixes merging two discard requests by having a special case for merging these using the unique constraints discards have. The second fixes merging a new discard bio into an existing request. Previously all the schedulers defaulted to a front merge, so the patch just checks merge type and adds a 'case' for handling discard in each of the callers. I've only been able to test with mq-deadline on NVMe as that is the only driver currently supporting multiple discard ranges in a single command. v1 -> v2: Fixed patch 1 based on feedback from Jens, tested with recreate sequence. Added 2nd patch for merging discard bios into existing requests. Keith Busch (2): block: Merge discard requests as a special case block: Handle merging discards in IO schedulers block/bfq-iosched.c | 2 +- block/blk-core.c | 4 ++++ block/blk-merge.c | 29 ++++++++++++++++++++++++++++- block/blk-mq-sched.c | 2 ++ block/cfq-iosched.c | 2 +- block/deadline-iosched.c | 4 +++- block/elevator.c | 2 +- block/mq-deadline.c | 2 +- 8 files changed, 41 insertions(+), 6 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 0/2] block: Discard merging fixes @ 2018-02-01 20:31 ` Keith Busch 0 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) This series fixes some incorrect accounting from merging discard requests. The first patch fixes merging two discard requests by having a special case for merging these using the unique constraints discards have. The second fixes merging a new discard bio into an existing request. Previously all the schedulers defaulted to a front merge, so the patch just checks merge type and adds a 'case' for handling discard in each of the callers. I've only been able to test with mq-deadline on NVMe as that is the only driver currently supporting multiple discard ranges in a single command. v1 -> v2: Fixed patch 1 based on feedback from Jens, tested with recreate sequence. Added 2nd patch for merging discard bios into existing requests. Keith Busch (2): block: Merge discard requests as a special case block: Handle merging discards in IO schedulers block/bfq-iosched.c | 2 +- block/blk-core.c | 4 ++++ block/blk-merge.c | 29 ++++++++++++++++++++++++++++- block/blk-mq-sched.c | 2 ++ block/cfq-iosched.c | 2 +- block/deadline-iosched.c | 4 +++- block/elevator.c | 2 +- block/mq-deadline.c | 2 +- 8 files changed, 41 insertions(+), 6 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 1/2] block: Merge discard requests as a special case 2018-02-01 20:31 ` Keith Busch @ 2018-02-01 20:31 ` Keith Busch -1 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) To: Linux Block, Linux NVMe, Christoph Hellwig, Jens Axboe; +Cc: Keith Busch Discard requests operate under different constraints than other operations and have different rules for merging. This patch will handle such requests as a special case, using the same criteria and segment accounting used for merging a discard bio into a reqseut. Signed-off-by: Keith Busch <keith.busch@intel.com> --- block/blk-merge.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/block/blk-merge.c b/block/blk-merge.c index 8452fc7164cc..e36462bc90f3 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -550,6 +550,25 @@ static bool req_no_special_merge(struct request *req) return !q->mq_ops && req->special; } +static int req_attempt_discard_merge(struct request_queue *q, struct request *req, + struct request *next) +{ + unsigned short segments = blk_rq_nr_discard_segments(req); + + if (segments >= queue_max_discard_segments(q)) + goto no_merge; + if (blk_rq_sectors(req) + bio_sectors(next->bio) > + blk_rq_get_max_sectors(req, blk_rq_pos(req))) + goto no_merge; + + req->nr_phys_segments = segments + blk_rq_nr_discard_segments(next); + return 1; + +no_merge: + req_set_nomerge(q, req); + return 0; +} + static int ll_merge_requests_fn(struct request_queue *q, struct request *req, struct request *next) { @@ -564,6 +583,13 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req, if (req_no_special_merge(req) || req_no_special_merge(next)) return 0; + /* + * Merging discard requests use different constraints than other + * operations. + */ + if (req_op(req) == REQ_OP_DISCARD) + return req_attempt_discard_merge(q, req, next); + if (req_gap_back_merge(req, next->bio)) return 0; @@ -715,7 +741,8 @@ static struct request *attempt_merge(struct request_queue *q, req->__data_len += blk_rq_bytes(next); - elv_merge_requests(q, req, next); + if (req_op(req) != REQ_OP_DISCARD) + elv_merge_requests(q, req, next); /* * 'next' is going away, so update stats accordingly -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 1/2] block: Merge discard requests as a special case @ 2018-02-01 20:31 ` Keith Busch 0 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) Discard requests operate under different constraints than other operations and have different rules for merging. This patch will handle such requests as a special case, using the same criteria and segment accounting used for merging a discard bio into a reqseut. Signed-off-by: Keith Busch <keith.busch at intel.com> --- block/blk-merge.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/block/blk-merge.c b/block/blk-merge.c index 8452fc7164cc..e36462bc90f3 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -550,6 +550,25 @@ static bool req_no_special_merge(struct request *req) return !q->mq_ops && req->special; } +static int req_attempt_discard_merge(struct request_queue *q, struct request *req, + struct request *next) +{ + unsigned short segments = blk_rq_nr_discard_segments(req); + + if (segments >= queue_max_discard_segments(q)) + goto no_merge; + if (blk_rq_sectors(req) + bio_sectors(next->bio) > + blk_rq_get_max_sectors(req, blk_rq_pos(req))) + goto no_merge; + + req->nr_phys_segments = segments + blk_rq_nr_discard_segments(next); + return 1; + +no_merge: + req_set_nomerge(q, req); + return 0; +} + static int ll_merge_requests_fn(struct request_queue *q, struct request *req, struct request *next) { @@ -564,6 +583,13 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req, if (req_no_special_merge(req) || req_no_special_merge(next)) return 0; + /* + * Merging discard requests use different constraints than other + * operations. + */ + if (req_op(req) == REQ_OP_DISCARD) + return req_attempt_discard_merge(q, req, next); + if (req_gap_back_merge(req, next->bio)) return 0; @@ -715,7 +741,8 @@ static struct request *attempt_merge(struct request_queue *q, req->__data_len += blk_rq_bytes(next); - elv_merge_requests(q, req, next); + if (req_op(req) != REQ_OP_DISCARD) + elv_merge_requests(q, req, next); /* * 'next' is going away, so update stats accordingly -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv2 1/2] block: Merge discard requests as a special case 2018-02-01 20:31 ` Keith Busch @ 2018-02-01 21:00 ` Jens Axboe -1 siblings, 0 replies; 12+ messages in thread From: Jens Axboe @ 2018-02-01 21:00 UTC (permalink / raw) To: Keith Busch, Linux Block, Linux NVMe, Christoph Hellwig On 2/1/18 1:31 PM, Keith Busch wrote: > Discard requests operate under different constraints than other operations > and have different rules for merging. This patch will handle such requests > as a special case, using the same criteria and segment accounting used > for merging a discard bio into a reqseut. I already fixed this one up. -- Jens Axboe ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 1/2] block: Merge discard requests as a special case @ 2018-02-01 21:00 ` Jens Axboe 0 siblings, 0 replies; 12+ messages in thread From: Jens Axboe @ 2018-02-01 21:00 UTC (permalink / raw) On 2/1/18 1:31 PM, Keith Busch wrote: > Discard requests operate under different constraints than other operations > and have different rules for merging. This patch will handle such requests > as a special case, using the same criteria and segment accounting used > for merging a discard bio into a reqseut. I already fixed this one up. -- Jens Axboe ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 2/2] block: Handle merging discards in IO schedulers 2018-02-01 20:31 ` Keith Busch @ 2018-02-01 20:31 ` Keith Busch -1 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) To: Linux Block, Linux NVMe, Christoph Hellwig, Jens Axboe; +Cc: Keith Busch This adds support for merging discard requests in all IO schedulers. Signed-off-by: Keith Busch <keith.busch@intel.com> --- block/bfq-iosched.c | 2 +- block/blk-core.c | 4 ++++ block/blk-mq-sched.c | 2 ++ block/cfq-iosched.c | 2 +- block/deadline-iosched.c | 4 +++- block/elevator.c | 2 +- block/mq-deadline.c | 2 +- 7 files changed, 13 insertions(+), 5 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 47e6ec7427c4..9b08c08298a6 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -1839,7 +1839,7 @@ static int bfq_request_merge(struct request_queue *q, struct request **req, __rq = bfq_find_rq_fmerge(bfqd, bio, q); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } return ELEVATOR_NO_MERGE; diff --git a/block/blk-core.c b/block/blk-core.c index a2005a485335..295ce3cbe1f6 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1952,6 +1952,10 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio) else elv_merged_request(q, req, ELEVATOR_FRONT_MERGE); goto out_unlock; + case ELEVATOR_DISCARD_MERGE: + if (!bio_attempt_discard_merge(q, req, bio)) + break; + goto out_unlock; default: break; } diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 55c0a745b427..25c14c58385c 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -259,6 +259,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio, if (!*merged_request) elv_merged_request(q, rq, ELEVATOR_FRONT_MERGE); return true; + case ELEVATOR_DISCARD_MERGE: + return bio_attempt_discard_merge(q, rq, bio); default: return false; } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 9f342ef1ad42..38158873d775 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2518,7 +2518,7 @@ static enum elv_merge cfq_merge(struct request_queue *q, struct request **req, __rq = cfq_find_rq_fmerge(cfqd, bio); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } return ELEVATOR_NO_MERGE; diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c index 9de9f156e203..849e03938653 100644 --- a/block/deadline-iosched.c +++ b/block/deadline-iosched.c @@ -14,6 +14,8 @@ #include <linux/compiler.h> #include <linux/rbtree.h> +#include "blk.h" + /* * See Documentation/block/deadline-iosched.txt */ @@ -142,7 +144,7 @@ deadline_merge(struct request_queue *q, struct request **req, struct bio *bio) if (elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } } } diff --git a/block/elevator.c b/block/elevator.c index e87e9b43aba0..7fd6cb9cfec1 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -484,7 +484,7 @@ enum elv_merge elv_merge(struct request_queue *q, struct request **req, __rq = elv_rqhash_find(q, bio->bi_iter.bi_sector); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_BACK_MERGE; + return blk_try_merge(__rq, bio); } if (e->uses_mq && e->type->ops.mq.request_merge) diff --git a/block/mq-deadline.c b/block/mq-deadline.c index c56f211c8440..a0f5752b6858 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -451,7 +451,7 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, if (elv_bio_merge_ok(__rq, bio)) { *rq = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } } -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv2 2/2] block: Handle merging discards in IO schedulers @ 2018-02-01 20:31 ` Keith Busch 0 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 20:31 UTC (permalink / raw) This adds support for merging discard requests in all IO schedulers. Signed-off-by: Keith Busch <keith.busch at intel.com> --- block/bfq-iosched.c | 2 +- block/blk-core.c | 4 ++++ block/blk-mq-sched.c | 2 ++ block/cfq-iosched.c | 2 +- block/deadline-iosched.c | 4 +++- block/elevator.c | 2 +- block/mq-deadline.c | 2 +- 7 files changed, 13 insertions(+), 5 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 47e6ec7427c4..9b08c08298a6 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -1839,7 +1839,7 @@ static int bfq_request_merge(struct request_queue *q, struct request **req, __rq = bfq_find_rq_fmerge(bfqd, bio, q); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } return ELEVATOR_NO_MERGE; diff --git a/block/blk-core.c b/block/blk-core.c index a2005a485335..295ce3cbe1f6 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1952,6 +1952,10 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio) else elv_merged_request(q, req, ELEVATOR_FRONT_MERGE); goto out_unlock; + case ELEVATOR_DISCARD_MERGE: + if (!bio_attempt_discard_merge(q, req, bio)) + break; + goto out_unlock; default: break; } diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 55c0a745b427..25c14c58385c 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -259,6 +259,8 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio, if (!*merged_request) elv_merged_request(q, rq, ELEVATOR_FRONT_MERGE); return true; + case ELEVATOR_DISCARD_MERGE: + return bio_attempt_discard_merge(q, rq, bio); default: return false; } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 9f342ef1ad42..38158873d775 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2518,7 +2518,7 @@ static enum elv_merge cfq_merge(struct request_queue *q, struct request **req, __rq = cfq_find_rq_fmerge(cfqd, bio); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } return ELEVATOR_NO_MERGE; diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c index 9de9f156e203..849e03938653 100644 --- a/block/deadline-iosched.c +++ b/block/deadline-iosched.c @@ -14,6 +14,8 @@ #include <linux/compiler.h> #include <linux/rbtree.h> +#include "blk.h" + /* * See Documentation/block/deadline-iosched.txt */ @@ -142,7 +144,7 @@ deadline_merge(struct request_queue *q, struct request **req, struct bio *bio) if (elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } } } diff --git a/block/elevator.c b/block/elevator.c index e87e9b43aba0..7fd6cb9cfec1 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -484,7 +484,7 @@ enum elv_merge elv_merge(struct request_queue *q, struct request **req, __rq = elv_rqhash_find(q, bio->bi_iter.bi_sector); if (__rq && elv_bio_merge_ok(__rq, bio)) { *req = __rq; - return ELEVATOR_BACK_MERGE; + return blk_try_merge(__rq, bio); } if (e->uses_mq && e->type->ops.mq.request_merge) diff --git a/block/mq-deadline.c b/block/mq-deadline.c index c56f211c8440..a0f5752b6858 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -451,7 +451,7 @@ static int dd_request_merge(struct request_queue *q, struct request **rq, if (elv_bio_merge_ok(__rq, bio)) { *rq = __rq; - return ELEVATOR_FRONT_MERGE; + return blk_try_merge(__rq, bio); } } -- 2.14.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv2 2/2] block: Handle merging discards in IO schedulers 2018-02-01 20:31 ` Keith Busch @ 2018-02-01 21:02 ` Jens Axboe -1 siblings, 0 replies; 12+ messages in thread From: Jens Axboe @ 2018-02-01 21:02 UTC (permalink / raw) To: Keith Busch, Linux Block, Linux NVMe, Christoph Hellwig On 2/1/18 1:31 PM, Keith Busch wrote: > This adds support for merging discard requests in all IO schedulers. As per my last email, some of these aren't correct. Only the blk-mq-sched change is correct/useful, so we should just do that separately. -- Jens Axboe ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 2/2] block: Handle merging discards in IO schedulers @ 2018-02-01 21:02 ` Jens Axboe 0 siblings, 0 replies; 12+ messages in thread From: Jens Axboe @ 2018-02-01 21:02 UTC (permalink / raw) On 2/1/18 1:31 PM, Keith Busch wrote: > This adds support for merging discard requests in all IO schedulers. As per my last email, some of these aren't correct. Only the blk-mq-sched change is correct/useful, so we should just do that separately. -- Jens Axboe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2 2/2] block: Handle merging discards in IO schedulers 2018-02-01 21:02 ` Jens Axboe @ 2018-02-01 21:28 ` Keith Busch -1 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 21:28 UTC (permalink / raw) To: Jens Axboe; +Cc: Linux Block, Linux NVMe, Christoph Hellwig On Thu, Feb 01, 2018 at 02:02:14PM -0700, Jens Axboe wrote: > On 2/1/18 1:31 PM, Keith Busch wrote: > > This adds support for merging discard requests in all IO schedulers. > > As per my last email, some of these aren't correct. Only the > blk-mq-sched change is correct/useful, so we should just do that > separately. Sorry, I got ahead of myself with this one. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 2/2] block: Handle merging discards in IO schedulers @ 2018-02-01 21:28 ` Keith Busch 0 siblings, 0 replies; 12+ messages in thread From: Keith Busch @ 2018-02-01 21:28 UTC (permalink / raw) On Thu, Feb 01, 2018@02:02:14PM -0700, Jens Axboe wrote: > On 2/1/18 1:31 PM, Keith Busch wrote: > > This adds support for merging discard requests in all IO schedulers. > > As per my last email, some of these aren't correct. Only the > blk-mq-sched change is correct/useful, so we should just do that > separately. Sorry, I got ahead of myself with this one. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-02-01 21:28 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-01 20:31 [PATCHv2 0/2] block: Discard merging fixes Keith Busch 2018-02-01 20:31 ` Keith Busch 2018-02-01 20:31 ` [PATCHv2 1/2] block: Merge discard requests as a special case Keith Busch 2018-02-01 20:31 ` Keith Busch 2018-02-01 21:00 ` Jens Axboe 2018-02-01 21:00 ` Jens Axboe 2018-02-01 20:31 ` [PATCHv2 2/2] block: Handle merging discards in IO schedulers Keith Busch 2018-02-01 20:31 ` Keith Busch 2018-02-01 21:02 ` Jens Axboe 2018-02-01 21:02 ` Jens Axboe 2018-02-01 21:28 ` Keith Busch 2018-02-01 21:28 ` Keith Busch
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.