Re: [PATCH 3/4] block/mq-deadline: fallback to per-cpu insertion buckets under contention

Bart Van Assche
2024-01-19T23:16:14Z

On 1/19/24 08:02, Jens Axboe wrote:
> If we attempt to insert a list of requests, but someone else is already
> running an insertion, then fallback to queueing that list internally and
> let the existing inserter finish the operation. The current inserter
> will either see and flush this list, of if it ends before we're done
> doing our bucket insert, then we'll flush it and insert ourselves.
>
> This reduces contention on the dd->lock, which protects any request
> insertion or dispatch, by having a backup point to insert into which
> will either be flushed immediately or by an existing inserter. As the
> alternative is to just keep spinning on the dd->lock, it's very easy
> to get into a situation where multiple processes are trying to do IO
> and all sit and spin on this lock.

With this alternative patch I achieve 20% higher IOPS than with patch
3/4 of this series for 1..4 CPU cores (null_blk + fio in an x86 VM):

---
block/mq-deadline.c | 61 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 49 insertions(+), 12 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 83bc21801226..fd9038a84dc5 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -89,11 +89,15 @@ struct deadline_data {
*/
struct {
spinlock_t lock;
+ spinlock_t insert_lock;
spinlock_t zone_lock;
} ____cacheline_aligned_in_smp;

unsigned long run_state;

+ struct list_head at_head;
+ struct list_head at_tail;
+
struct dd_per_prio per_prio[DD_PRIO_COUNT];

/* Data direction of latest dispatched request. */
@@ -120,6 +124,9 @@ static const enum dd_prio ioprio_class_to_prio[] = {
[IOPRIO_CLASS_IDLE] = DD_IDLE_PRIO,
};

+static void dd_insert_request(struct request_queue *q, struct request *rq,
+ blk_insert_t flags, struct list_head *free);
+
static inline struct rb_root *
deadline_rb_root(struct dd_per_prio *per_prio, struct request *rq)
{
@@ -592,6 +599,30 @@ static struct request *dd_dispatch_prio_aged_requests(struct deadline_data *dd,
return NULL;
}

+static void dd_do_insert(struct request_queue *q, struct list_head *free)
+{
+ struct deadline_data *dd = q->elevator->elevator_data;
+ struct request *rq;
+ LIST_HEAD(at_head);
+ LIST_HEAD(at_tail);
+
+ spin_lock(&dd->insert_lock);
+ list_splice_init(&dd->at_head, &at_head);
+ list_splice_init(&dd->at_tail, &at_tail);
+ spin_unlock(&dd->insert_lock);
+
+ while (!list_empty(&at_head)) {
+ rq = list_first_entry(&at_head, struct request, queuelist);
+ list_del_init(&rq->queuelist);
+ dd_insert_request(q, rq, BLK_MQ_INSERT_AT_HEAD, free);
+ }
+ while (!list_empty(&at_tail)) {
+ rq = list_first_entry(&at_tail, struct request, queuelist);
+ list_del_init(&rq->queuelist);
+ dd_insert_request(q, rq, 0, free);
+ }
+}
+
/*
* Called from blk_mq_run_hw_queue() -> __blk_mq_sched_dispatch_requests().
*
@@ -606,6 +637,7 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
const unsigned long now = jiffies;
struct request *rq;
enum dd_prio prio;
+ LIST_HEAD(free);

/*
* If someone else is already dispatching, skip this one. This will
@@ -620,6 +652,7 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
return NULL;

spin_lock(&dd->lock);
+ dd_do_insert(hctx->queue, &free);
rq = dd_dispatch_prio_aged_requests(dd, now);
if (rq)
goto unlock;
@@ -638,6 +671,8 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
clear_bit(DD_DISPATCHING, &dd->run_state);
spin_unlock(&dd->lock);

+ blk_mq_free_requests(&free);
+
return rq;
}

@@ -727,8 +762,12 @@ static int dd_init_sched(struct request_queue *q, struct elevator_type *e)
eq->elevator_data = dd;

spin_lock_init(&dd->lock);
+ spin_lock_init(&dd->insert_lock);
spin_lock_init(&dd->zone_lock);

+ INIT_LIST_HEAD(&dd->at_head);
+ INIT_LIST_HEAD(&dd->at_tail);
+
for (prio = 0; prio <= DD_PRIO_MAX; prio++) {
struct dd_per_prio *per_prio = &dd->per_prio[prio];

@@ -899,19 +938,13 @@ static void dd_insert_requests(struct blk_mq_hw_ctx *hctx,
{
struct request_queue *q = hctx->queue;
struct deadline_data *dd = q->elevator->elevator_data;
- LIST_HEAD(free);
-
- spin_lock(&dd->lock);
- while (!list_empty(list)) {
- struct request *rq;

- rq = list_first_entry(list, struct request, queuelist);
- list_del_init(&rq->queuelist);
- dd_insert_request(q, rq, flags, &free);
- }
- spin_unlock(&dd->lock);
-
- blk_mq_free_requests(&free);
+ spin_lock(&dd->insert_lock);
+ if (flags & BLK_MQ_INSERT_AT_HEAD)
+ list_splice_init(list, &dd->at_head);
+ else
+ list_splice_init(list, &dd->at_tail);
+ spin_unlock(&dd->insert_lock);
}

/* Callback from inside blk_mq_rq_ctx_init(). */
@@ -990,6 +1023,10 @@ static bool dd_has_work(struct blk_mq_hw_ctx *hctx)
struct deadline_data *dd = hctx->queue->elevator->elevator_data;
enum dd_prio prio;

+ if (!list_empty_careful(&dd->at_head) ||
+ !list_empty_careful(&dd->at_tail))
+ return true;
+
for (prio = 0; prio <= DD_PRIO_MAX; prio++)
if (dd_has_work_for_prio(&dd->per_prio[prio]))
return true;