* [WIP PATCHSET 0/4] WIP branch for bfq-mq
From: Paolo Valente @ 2017-02-07 17:24 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij, broonie,
Paolo Valente
Hi,
I have finally pushed here [1] the current WIP branch of bfq for
blk-mq, which I have tentatively named bfq-mq.
This branch *IS NOT* meant for merging into mainline and contain code
that mau easily violate code style, and not only, in many
places. Commits implement the following main steps:
1) Add the last version of bfq for blk
2) Clone bfq source files into identical bfq-mq source files
3) Modify bfq-mq files to get a working version of bfq for blk-mq
(cgroups support not yet functional)
In my intentions, the main goals of this branch are:
1) Show, as soon as I could, the changes I made to let bfq-mq comply
with blk-mq-sched framework. I though this could be particularly
useful for Jens, being BFQ identical to CFQ in terms of hook
interfaces and io-context handling, and almost identical in terms
request-merging.
2) Enable people to test this first version bfq-mq. Code is purposely
overfull of log messages and invariant checks that halt the system on
failure (lock assertions, BUG_ONs, ...).
To make it easier to revise commits, I'm sending the patches that
transform bfq into bfq-mq (last four patches in the branch [1]). They
work on two files, bfq-mq-iosched.c and bfq-mq.h, which, at the
beginning, are just copies of bfq-iosched.c and bfq.h.
Thanks,
Paolo
[1] https://github.com/Algodev-github/bfq-mq
Paolo Valente (4):
blk-mq: pass bio to blk_mq_sched_get_rq_priv
Move thinktime from bic to bfqq
Embed bfq-ioc.c and add locking on request queue
Modify interface and operation to comply with blk-mq-sched
block/bfq-cgroup.c | 4 -
block/bfq-mq-iosched.c | 852 +++++++++++++++++++++++++++++------------------
block/bfq-mq.h | 65 ++--
block/blk-mq-sched.c | 8 +-
block/blk-mq-sched.h | 5 +-
include/linux/elevator.h | 2 +-
6 files changed, 567 insertions(+), 369 deletions(-)
--
2.10.0
^ permalink raw reply
* [WIP PATCHSET 4/4] Modify interface and operation to comply with blk-mq-sched
From: Paolo Valente @ 2017-02-07 17:24 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij, broonie,
Paolo Valente
In-Reply-To: <20170207172446.4528-1-paolo.valente@linaro.org>
As for modifications of the operation, the major changes are the introduction
of a scheduler lock, and the moving to deferred work of the body of the hook
exit_icq. The latter change has been made to avoid deadlocks caused by the
combination of the following facts: 1) such a body takes the scheduler lock,
and, if not deferred, 2) it does so from inside the exit_icq hook, which is
invoked with the queue lock held, and 3) there is at least one code path,
namely that starting from bfq_bio_merge, which takes these locks in the
opposite order.
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
block/bfq-cgroup.c | 4 -
block/bfq-mq-iosched.c | 791 ++++++++++++++++++++++++++++++-------------------
block/bfq-mq.h | 37 +--
3 files changed, 496 insertions(+), 336 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 36b68ec..7ecce47 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -472,8 +472,6 @@ static struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,
struct bfq_group *bfqg, *parent;
struct bfq_entity *entity;
- assert_spin_locked(bfqd->queue->queue_lock);
-
bfqg = bfq_lookup_bfqg(bfqd, blkcg);
if (unlikely(!bfqg))
@@ -602,8 +600,6 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
struct bfq_group *bfqg;
struct bfq_entity *entity;
- lockdep_assert_held(bfqd->queue->queue_lock);
-
bfqg = bfq_find_set_group(bfqd, blkcg);
if (unlikely(!bfqg))
diff --git a/block/bfq-mq-iosched.c b/block/bfq-mq-iosched.c
index a8679de..05a12b6 100644
--- a/block/bfq-mq-iosched.c
+++ b/block/bfq-mq-iosched.c
@@ -76,9 +76,14 @@
#include <linux/jiffies.h>
#include <linux/rbtree.h>
#include <linux/ioprio.h>
-#undef CONFIG_BFQ_GROUP_IOSCHED /* cgroups support not yet functional */
-#include "bfq.h"
+#include <linux/sbitmap.h>
+#include <linux/delay.h>
+
#include "blk.h"
+#include "blk-mq.h"
+#include "blk-mq-tag.h"
+#include "blk-mq-sched.h"
+#include "bfq-mq.h"
/* Expiration time of sync (0) and async (1) requests, in ns. */
static const u64 bfq_fifo_expire[2] = { NSEC_PER_SEC / 4, NSEC_PER_SEC / 8 };
@@ -188,8 +193,6 @@ static int device_speed_thresh[2];
#define RQ_BIC(rq) ((struct bfq_io_cq *) (rq)->elv.priv[0])
#define RQ_BFQQ(rq) ((rq)->elv.priv[1])
-static void bfq_schedule_dispatch(struct bfq_data *bfqd);
-
/**
* icq_to_bic - convert iocontext queue structure to bfq_io_cq.
* @icq: the iocontext queue.
@@ -211,11 +214,12 @@ static struct bfq_io_cq *bfq_bic_lookup(struct bfq_data *bfqd,
struct request_queue *q)
{
if (ioc) {
+ unsigned long flags;
struct bfq_io_cq *icq;
- spin_lock_irq(q->queue_lock);
+ spin_lock_irqsave(q->queue_lock, flags);
icq = icq_to_bic(ioc_lookup_icq(ioc, q));
- spin_unlock_irq(q->queue_lock);
+ spin_unlock_irqrestore(q->queue_lock, flags);
return icq;
}
@@ -239,7 +243,7 @@ static void bfq_schedule_dispatch(struct bfq_data *bfqd)
{
if (bfqd->queued != 0) {
bfq_log(bfqd, "schedule dispatch");
- kblockd_schedule_work(&bfqd->unplug_work);
+ blk_mq_run_hw_queues(bfqd->queue, true);
}
}
@@ -728,9 +732,9 @@ static int bfqq_process_refs(struct bfq_queue *bfqq)
{
int process_refs, io_refs;
- lockdep_assert_held(bfqq->bfqd->queue->queue_lock);
+ lockdep_assert_held(&bfqq->bfqd->lock);
- io_refs = bfqq->allocated[READ] + bfqq->allocated[WRITE];
+ io_refs = bfqq->allocated;
process_refs = bfqq->ref - io_refs - bfqq->entity.on_st;
BUG_ON(process_refs < 0);
return process_refs;
@@ -1441,6 +1445,8 @@ static void bfq_add_request(struct request *rq)
bfqq->queued[rq_is_sync(rq)]++;
bfqd->queued++;
+ BUG_ON(!RQ_BFQQ(rq));
+ BUG_ON(RQ_BFQQ(rq) != bfqq);
elv_rb_add(&bfqq->sort_list, rq);
/*
@@ -1449,6 +1455,8 @@ static void bfq_add_request(struct request *rq)
prev = bfqq->next_rq;
next_rq = bfq_choose_req(bfqd, bfqq->next_rq, rq, bfqd->last_position);
BUG_ON(!next_rq);
+ BUG_ON(!RQ_BFQQ(next_rq));
+ BUG_ON(RQ_BFQQ(next_rq) != bfqq);
bfqq->next_rq = next_rq;
/*
@@ -1544,6 +1552,7 @@ static sector_t get_sdist(sector_t last_pos, struct request *rq)
return sdist;
}
+#if 0 /* Still not clear if we can do without next two functions */
static void bfq_activate_request(struct request_queue *q, struct request *rq)
{
struct bfq_data *bfqd = q->elevator->elevator_data;
@@ -1557,8 +1566,10 @@ static void bfq_deactivate_request(struct request_queue *q, struct request *rq)
BUG_ON(bfqd->rq_in_driver == 0);
bfqd->rq_in_driver--;
}
+#endif
-static void bfq_remove_request(struct request *rq)
+static void bfq_remove_request(struct request_queue *q,
+ struct request *rq)
{
struct bfq_queue *bfqq = RQ_BFQQ(rq);
struct bfq_data *bfqd = bfqq->bfqd;
@@ -1569,6 +1580,19 @@ static void bfq_remove_request(struct request *rq)
if (bfqq->next_rq == rq) {
bfqq->next_rq = bfq_find_next_rq(bfqd, bfqq, rq);
+ if (bfqq->next_rq && !RQ_BFQQ(bfqq->next_rq)) {
+ pr_crit("no bfqq! for next rq %p bfqq %p\n",
+ bfqq->next_rq, bfqq);
+ }
+
+ BUG_ON(bfqq->next_rq && !RQ_BFQQ(bfqq->next_rq));
+ if (bfqq->next_rq && RQ_BFQQ(bfqq->next_rq) != bfqq) {
+ pr_crit(
+ "wrong bfqq! for next rq %p, rq_bfqq %p bfqq %p\n",
+ bfqq->next_rq, RQ_BFQQ(bfqq->next_rq), bfqq);
+ }
+ BUG_ON(bfqq->next_rq && RQ_BFQQ(bfqq->next_rq) != bfqq);
+
bfq_updated_next_req(bfqd, bfqq);
}
@@ -1579,6 +1603,10 @@ static void bfq_remove_request(struct request *rq)
bfqd->queued--;
elv_rb_del(&bfqq->sort_list, rq);
+ elv_rqhash_del(q, rq);
+ if (q->last_merge == rq)
+ q->last_merge = NULL;
+
if (RB_EMPTY_ROOT(&bfqq->sort_list)) {
bfqq->next_rq = NULL;
@@ -1616,22 +1644,47 @@ static void bfq_remove_request(struct request *rq)
bfqg_stats_update_io_remove(bfqq_group(bfqq), rq->cmd_flags);
}
-static int bfq_merge(struct request_queue *q, struct request **req,
- struct bio *bio)
+static bool bfq_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
+{
+ struct request_queue *q = hctx->queue;
+ struct bfq_data *bfqd = q->elevator->elevator_data;
+ struct request *free = NULL;
+ bool ret;
+
+ spin_lock_irq(&bfqd->lock);
+ ret = blk_mq_sched_try_merge(q, bio, &free);
+
+ /*
+ * XXX Not yet freeing without lock held, to avoid an
+ * inconsistency with respect to the lock-protected invocation
+ * of blk_mq_sched_try_insert_merge in bfq_bio_merge. Waiting
+ * for clarifications from Jens.
+ */
+ if (free)
+ blk_mq_free_request(free);
+ spin_unlock_irq(&bfqd->lock);
+
+ return ret;
+}
+
+static int bfq_request_merge(struct request_queue *q, struct request **req,
+ struct bio *bio)
{
struct bfq_data *bfqd = q->elevator->elevator_data;
struct request *__rq;
- __rq = bfq_find_rq_fmerge(bfqd, bio);
+ __rq = bfq_find_rq_fmerge(bfqd, bio, q);
if (__rq && elv_bio_merge_ok(__rq, bio)) {
*req = __rq;
+ bfq_log(bfqd, "request_merge: req %p", __rq);
+
return ELEVATOR_FRONT_MERGE;
}
return ELEVATOR_NO_MERGE;
}
-static void bfq_merged_request(struct request_queue *q, struct request *req,
+static void bfq_request_merged(struct request_queue *q, struct request *req,
int type)
{
if (type == ELEVATOR_FRONT_MERGE &&
@@ -1645,13 +1698,23 @@ static void bfq_merged_request(struct request_queue *q, struct request *req,
/* Reposition request in its sort_list */
elv_rb_del(&bfqq->sort_list, req);
+ BUG_ON(!RQ_BFQQ(req));
+ BUG_ON(RQ_BFQQ(req) != bfqq);
elv_rb_add(&bfqq->sort_list, req);
+
+ spin_lock_irq(&bfqd->lock);
/* Choose next request to be served for bfqq */
prev = bfqq->next_rq;
next_rq = bfq_choose_req(bfqd, bfqq->next_rq, req,
bfqd->last_position);
BUG_ON(!next_rq);
+
bfqq->next_rq = next_rq;
+
+ bfq_log_bfqq(bfqd, bfqq,
+ "requests_merged: req %p prev %p next_rq %p bfqq %p",
+ req, prev, next_rq, bfqq);
+
/*
* If next_rq changes, update both the queue's budget to
* fit the new request and the queue's position in its
@@ -1661,22 +1724,27 @@ static void bfq_merged_request(struct request_queue *q, struct request *req,
bfq_updated_next_req(bfqd, bfqq);
bfq_pos_tree_add_move(bfqd, bfqq);
}
+ spin_unlock_irq(&bfqd->lock);
}
}
-#ifdef BFQ_GROUP_IOSCHED_ENABLED
-static void bfq_bio_merged(struct request_queue *q, struct request *req,
- struct bio *bio)
-{
- bfqg_stats_update_io_merged(bfqq_group(RQ_BFQQ(req)), bio->bi_opf);
-}
-#endif
-
-static void bfq_merged_requests(struct request_queue *q, struct request *rq,
+static void bfq_requests_merged(struct request_queue *q, struct request *rq,
struct request *next)
{
struct bfq_queue *bfqq = RQ_BFQQ(rq), *next_bfqq = RQ_BFQQ(next);
+ BUG_ON(!RQ_BFQQ(rq));
+ BUG_ON(!RQ_BFQQ(next));
+
+ if (!RB_EMPTY_NODE(&rq->rb_node))
+ goto end;
+
+ bfq_log_bfqq(bfqq->bfqd, bfqq,
+ "requests_merged: rq %p next %p bfqq %p next_bfqq %p",
+ rq, next, bfqq, next_bfqq);
+
+ spin_lock_irq(&bfqq->bfqd->lock);
+
/*
* If next and rq belong to the same bfq_queue and next is older
* than rq, then reposition rq in the fifo (by substituting next
@@ -1697,7 +1765,10 @@ static void bfq_merged_requests(struct request_queue *q, struct request *rq,
if (bfqq->next_rq == next)
bfqq->next_rq = rq;
- bfq_remove_request(next);
+ bfq_remove_request(q, next);
+
+ spin_unlock_irq(&bfqq->bfqd->lock);
+end:
bfqg_stats_update_io_merged(bfqq_group(bfqq), next->cmd_flags);
}
@@ -1741,7 +1812,7 @@ static void bfq_end_wr(struct bfq_data *bfqd)
{
struct bfq_queue *bfqq;
- spin_lock_irq(bfqd->queue->queue_lock);
+ spin_lock_irq(&bfqd->lock);
list_for_each_entry(bfqq, &bfqd->active_list, bfqq_list)
bfq_bfqq_end_wr(bfqq);
@@ -1749,7 +1820,7 @@ static void bfq_end_wr(struct bfq_data *bfqd)
bfq_bfqq_end_wr(bfqq);
bfq_end_wr_async(bfqd);
- spin_unlock_irq(bfqd->queue->queue_lock);
+ spin_unlock_irq(&bfqd->lock);
}
static sector_t bfq_io_struct_pos(void *io_struct, bool request)
@@ -2131,8 +2202,8 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic,
bfq_put_queue(bfqq);
}
-static int bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
- struct bio *bio)
+static bool bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
+ struct bio *bio)
{
struct bfq_data *bfqd = q->elevator->elevator_data;
bool is_sync = op_is_sync(bio->bi_opf);
@@ -2150,7 +2221,7 @@ static int bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
* merge only if rq is queued there.
* Queue lock is held here.
*/
- bic = bfq_bic_lookup(bfqd, current->io_context);
+ bic = bfq_bic_lookup(bfqd, current->io_context, q);
if (!bic)
return false;
@@ -2175,12 +2246,6 @@ static int bfq_allow_bio_merge(struct request_queue *q, struct request *rq,
return bfqq == RQ_BFQQ(rq);
}
-static int bfq_allow_rq_merge(struct request_queue *q, struct request *rq,
- struct request *next)
-{
- return RQ_BFQQ(rq) == RQ_BFQQ(next);
-}
-
/*
* Set the maximum time for the in-service queue to consume its
* budget. This prevents seeky processes from lowering the throughput.
@@ -2211,7 +2276,6 @@ static void __bfq_set_in_service_queue(struct bfq_data *bfqd,
{
if (bfqq) {
bfqg_stats_update_avg_queue_size(bfqq_group(bfqq));
- bfq_mark_bfqq_must_alloc(bfqq);
bfq_clear_bfqq_fifo_expire(bfqq);
bfqd->budgets_assigned = (bfqd->budgets_assigned*7 + 256) / 8;
@@ -2650,27 +2714,28 @@ static void bfq_update_peak_rate(struct bfq_data *bfqd, struct request *rq)
}
/*
- * Move request from internal lists to the dispatch list of the request queue
+ * Remove request from internal lists.
*/
-static void bfq_dispatch_insert(struct request_queue *q, struct request *rq)
+static void bfq_dispatch_remove(struct request_queue *q, struct request *rq)
{
struct bfq_queue *bfqq = RQ_BFQQ(rq);
/*
- * For consistency, the next instruction should have been executed
- * after removing the request from the queue and dispatching it.
- * We execute instead this instruction before bfq_remove_request()
- * (and hence introduce a temporary inconsistency), for efficiency.
- * In fact, in a forced_dispatch, this prevents two counters related
- * to bfqq->dispatched to risk to be uselessly decremented if bfqq
- * is not in service, and then to be incremented again after
- * incrementing bfqq->dispatched.
+ * For consistency, the next instruction should have been
+ * executed after removing the request from the queue and
+ * dispatching it. We execute instead this instruction before
+ * bfq_remove_request() (and hence introduce a temporary
+ * inconsistency), for efficiency. In fact, should this
+ * dispatch occur for a non in-service bfqq, this anticipated
+ * increment prevents two counters related to bfqq->dispatched
+ * from risking to be, first, uselessly decremented, and then
+ * incremented again when the (new) value of bfqq->dispatched
+ * happens to be taken into account.
*/
bfqq->dispatched++;
bfq_update_peak_rate(q->elevator->elevator_data, rq);
- bfq_remove_request(rq);
- elv_dispatch_sort(q, rq);
+ bfq_remove_request(q, rq);
}
static void __bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq)
@@ -3534,7 +3599,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
bfq_log_bfqq(bfqd, bfqq, "select_queue: already in-service queue");
if (bfq_may_expire_for_budg_timeout(bfqq) &&
- !hrtimer_active(&bfqd->idle_slice_timer) &&
+ !bfq_bfqq_wait_request(bfqq) &&
!bfq_bfqq_must_idle(bfqq))
goto expire;
@@ -3570,7 +3635,6 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
* arrives.
*/
if (bfq_bfqq_wait_request(bfqq)) {
- BUG_ON(!hrtimer_active(&bfqd->idle_slice_timer));
/*
* If we get here: 1) at least a new request
* has arrived but we have not disabled the
@@ -3597,7 +3661,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
* for a new request, or has requests waiting for a completion and
* may idle after their completion, then keep it anyway.
*/
- if (hrtimer_active(&bfqd->idle_slice_timer) ||
+ if (bfq_bfqq_wait_request(bfqq) ||
(bfqq->dispatched != 0 && bfq_bfqq_may_idle(bfqq))) {
bfqq = NULL;
goto keep_queue;
@@ -3676,13 +3740,11 @@ static void bfq_update_wr_data(struct bfq_data *bfqd, struct bfq_queue *bfqq)
}
/*
- * Dispatch one request from bfqq, moving it to the request queue
- * dispatch list.
+ * Dispatch next request from bfqq.
*/
-static int bfq_dispatch_request(struct bfq_data *bfqd,
- struct bfq_queue *bfqq)
+static struct request *bfq_dispatch_rq_from_bfqq(struct bfq_data *bfqd,
+ struct bfq_queue *bfqq)
{
- int dispatched = 0;
struct request *rq = bfqq->next_rq;
unsigned long service_to_charge;
@@ -3698,7 +3760,7 @@ static int bfq_dispatch_request(struct bfq_data *bfqd,
BUG_ON(bfqq->entity.budget < bfqq->entity.service);
- bfq_dispatch_insert(bfqd->queue, rq);
+ bfq_dispatch_remove(bfqd->queue, rq);
/*
* If weight raising has to terminate for bfqq, then next
@@ -3714,86 +3776,66 @@ static int bfq_dispatch_request(struct bfq_data *bfqd,
bfq_update_wr_data(bfqd, bfqq);
bfq_log_bfqq(bfqd, bfqq,
- "dispatched %u sec req (%llu), budg left %d",
+ "dispatched %u sec req (%llu), budg left %d, new disp_nr %d",
blk_rq_sectors(rq),
(unsigned long long) blk_rq_pos(rq),
- bfq_bfqq_budget_left(bfqq));
-
- dispatched++;
+ bfq_bfqq_budget_left(bfqq),
+ bfqq->dispatched);
if (!bfqd->in_service_bic) {
atomic_long_inc(&RQ_BIC(rq)->icq.ioc->refcount);
bfqd->in_service_bic = RQ_BIC(rq);
}
+ /*
+ * Expire bfqq, pretending that its budget expired, if bfqq
+ * belongs to CLASS_IDLE and other queues are waiting for
+ * service.
+ */
if (bfqd->busy_queues > 1 && bfq_class_idle(bfqq))
goto expire;
- return dispatched;
+ return rq;
expire:
bfq_bfqq_expire(bfqd, bfqq, false, BFQ_BFQQ_BUDGET_EXHAUSTED);
- return dispatched;
-}
-
-static int __bfq_forced_dispatch_bfqq(struct bfq_queue *bfqq)
-{
- int dispatched = 0;
-
- while (bfqq->next_rq) {
- bfq_dispatch_insert(bfqq->bfqd->queue, bfqq->next_rq);
- dispatched++;
- }
-
- BUG_ON(!list_empty(&bfqq->fifo));
- return dispatched;
+ return rq;
}
-/*
- * Drain our current requests.
- * Used for barriers and when switching io schedulers on-the-fly.
- */
-static int bfq_forced_dispatch(struct bfq_data *bfqd)
+static bool bfq_has_work(struct blk_mq_hw_ctx *hctx)
{
- struct bfq_queue *bfqq, *n;
- struct bfq_service_tree *st;
- int dispatched = 0;
+ struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
- bfqq = bfqd->in_service_queue;
- if (bfqq)
- __bfq_bfqq_expire(bfqd, bfqq);
+ bfq_log(bfqd, "has_work, dispatch_non_empty %d busy_queues %d",
+ !list_empty_careful(&bfqd->dispatch), bfqd->busy_queues > 0);
/*
- * Loop through classes, and be careful to leave the scheduler
- * in a consistent state, as feedback mechanisms and vtime
- * updates cannot be disabled during the process.
+ * Avoiding lock: a race on bfqd->busy_queues should cause at
+ * most a call to dispatch for nothing
*/
- list_for_each_entry_safe(bfqq, n, &bfqd->active_list, bfqq_list) {
- st = bfq_entity_service_tree(&bfqq->entity);
-
- dispatched += __bfq_forced_dispatch_bfqq(bfqq);
-
- bfqq->max_budget = bfq_max_budget(bfqd);
- bfq_forget_idle(st);
- }
-
- BUG_ON(bfqd->busy_queues != 0);
-
- return dispatched;
+ return !list_empty_careful(&bfqd->dispatch) ||
+ bfqd->busy_queues > 0;
}
-static int bfq_dispatch_requests(struct request_queue *q, int force)
+static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx)
{
- struct bfq_data *bfqd = q->elevator->elevator_data;
- struct bfq_queue *bfqq;
+ struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
+ struct request *rq = NULL;
+ struct bfq_queue *bfqq = NULL;
+
+ if (!list_empty(&bfqd->dispatch)) {
+ rq = list_first_entry(&bfqd->dispatch, struct request,
+ queuelist);
+ list_del_init(&rq->queuelist);
+ bfq_log(bfqd,
+ "dispatch requests: picked %p from dispatch list", rq);
+ goto exit;
+ }
bfq_log(bfqd, "dispatch requests: %d busy queues", bfqd->busy_queues);
if (bfqd->busy_queues == 0)
- return 0;
-
- if (unlikely(force))
- return bfq_forced_dispatch(bfqd);
+ goto exit;
/*
* Force device to serve one request at a time if
@@ -3808,25 +3850,53 @@ static int bfq_dispatch_requests(struct request_queue *q, int force)
* throughput.
*/
if (bfqd->strict_guarantees && bfqd->rq_in_driver > 0)
- return 0;
+ goto exit;
bfqq = bfq_select_queue(bfqd);
if (!bfqq)
- return 0;
+ goto exit;
BUG_ON(bfqq->entity.budget < bfqq->entity.service);
BUG_ON(bfq_bfqq_wait_request(bfqq));
- if (!bfq_dispatch_request(bfqd, bfqq))
- return 0;
-
- bfq_log_bfqq(bfqd, bfqq, "dispatched %s request",
- bfq_bfqq_sync(bfqq) ? "sync" : "async");
+ rq = bfq_dispatch_rq_from_bfqq(bfqd, bfqq);
BUG_ON(bfqq->next_rq == NULL &&
bfqq->entity.budget < bfqq->entity.service);
- return 1;
+exit:
+ if (rq) {
+ rq->rq_flags |= RQF_STARTED;
+ bfqd->rq_in_driver++;
+ if (bfqq)
+ bfq_log_bfqq(bfqd, bfqq,
+ "dispatched %s request %p, rq_in_driver %d",
+ bfq_bfqq_sync(bfqq) ? "sync" : "async",
+ rq,
+ bfqd->rq_in_driver);
+ else
+ bfq_log(bfqd,
+ "dispatched request %p from dispatch list, rq_in_driver %d",
+ rq, bfqd->rq_in_driver);
+ } else
+ bfq_log(bfqd,
+ "returned NULL request, rq_in_driver %d",
+ bfqd->rq_in_driver);
+
+ return rq;
+}
+
+
+static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx)
+{
+ struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
+ struct request *rq;
+
+ spin_lock_irq(&bfqd->lock);
+ rq = __bfq_dispatch_request(hctx);
+ spin_unlock_irq(&bfqd->lock);
+
+ return rq;
}
/*
@@ -3843,13 +3913,15 @@ static void bfq_put_queue(struct bfq_queue *bfqq)
BUG_ON(bfqq->ref <= 0);
- bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d", bfqq, bfqq->ref);
+ if (bfqq->bfqd)
+ bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p %d", bfqq, bfqq->ref);
+
bfqq->ref--;
if (bfqq->ref)
return;
BUG_ON(rb_first(&bfqq->sort_list));
- BUG_ON(bfqq->allocated[READ] + bfqq->allocated[WRITE] != 0);
+ BUG_ON(bfqq->allocated != 0);
BUG_ON(bfqq->entity.tree);
BUG_ON(bfq_bfqq_busy(bfqq));
@@ -3864,7 +3936,8 @@ static void bfq_put_queue(struct bfq_queue *bfqq)
*/
hlist_del_init(&bfqq->burst_list_node);
- bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p freed", bfqq);
+ if (bfqq->bfqd)
+ bfq_log_bfqq(bfqq->bfqd, bfqq, "put_queue: %p freed", bfqq);
kmem_cache_free(bfq_pool, bfqq);
#ifdef BFQ_GROUP_IOSCHED_ENABLED
@@ -3905,29 +3978,53 @@ static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq)
bfq_put_queue(bfqq);
}
-static void bfq_exit_icq(struct io_cq *icq)
+static void bfq_exit_icq_bfqq(struct bfq_io_cq *bic, bool is_sync)
{
- struct bfq_io_cq *bic = icq_to_bic(icq);
- struct bfq_data *bfqd = bic_to_bfqd(bic);
+ struct bfq_queue *bfqq = bic_to_bfqq(bic, is_sync);
+ struct bfq_data *bfqd;
- if (bic_to_bfqq(bic, false)) {
- bfq_exit_bfqq(bfqd, bic_to_bfqq(bic, false));
- bic_set_bfqq(bic, NULL, false);
- }
+ if (bfqq)
+ bfqd = bfqq->bfqd; /* NULL if scheduler already exited */
- if (bic_to_bfqq(bic, true)) {
+ if (bfqq && bfqd) {
+ spin_lock_irq(&bfqd->lock);
/*
* If the bic is using a shared queue, put the reference
* taken on the io_context when the bic started using a
* shared bfq_queue.
*/
- if (bfq_bfqq_coop(bic_to_bfqq(bic, true)))
- put_io_context(icq->ioc);
- bfq_exit_bfqq(bfqd, bic_to_bfqq(bic, true));
- bic_set_bfqq(bic, NULL, true);
+ if (is_sync && bfq_bfqq_coop(bfqq))
+ put_io_context(bic->icq.ioc);
+ bfq_exit_bfqq(bfqd, bfqq);
+ bic_set_bfqq(bic, NULL, is_sync);
+ spin_unlock_irq(&bfqd->lock);
}
}
+static void bfq_exit_icq_body(struct work_struct *work)
+{
+ struct bfq_io_cq *bic =
+ container_of(work, struct bfq_io_cq, exit_icq_work);
+
+ bfq_exit_icq_bfqq(bic, true);
+ bfq_exit_icq_bfqq(bic, false);
+}
+
+static void bfq_init_icq(struct io_cq *icq)
+{
+ struct bfq_io_cq *bic = icq_to_bic(icq);
+
+ INIT_WORK(&bic->exit_icq_work, bfq_exit_icq_body);
+}
+
+static void bfq_exit_icq(struct io_cq *icq)
+{
+ struct bfq_io_cq *bic = icq_to_bic(icq);
+
+ BUG_ON(!bic);
+ kblockd_schedule_work(&bic->exit_icq_work);
+}
+
/*
* Update the entity prio values; note that the new values will not
* be used until the next (re)activation.
@@ -3937,6 +4034,11 @@ static void bfq_set_next_ioprio_data(struct bfq_queue *bfqq,
{
struct task_struct *tsk = current;
int ioprio_class;
+ struct bfq_data *bfqd = bfqq->bfqd;
+
+ WARN_ON(!bfqd);
+ if (!bfqd)
+ return;
ioprio_class = IOPRIO_PRIO_CLASS(bic->ioprio);
switch (ioprio_class) {
@@ -4017,6 +4119,8 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
INIT_HLIST_NODE(&bfqq->burst_list_node);
BUG_ON(!hlist_unhashed(&bfqq->burst_list_node));
+ spin_lock_init(&bfqq->lock);
+
bfqq->ref = 0;
bfqq->bfqd = bfqd;
@@ -4273,21 +4377,17 @@ static void bfq_rq_enqueued(struct bfq_data *bfqd, struct bfq_queue *bfqq,
if (budget_timeout)
bfq_bfqq_expire(bfqd, bfqq, false,
BFQ_BFQQ_BUDGET_TIMEOUT);
-
- /*
- * Let the request rip immediately, or let a new queue be
- * selected if bfqq has just been expired.
- */
- __blk_run_queue(bfqd->queue);
}
}
-static void bfq_insert_request(struct request_queue *q, struct request *rq)
+
+static void __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
{
- struct bfq_data *bfqd = q->elevator->elevator_data;
struct bfq_queue *bfqq = RQ_BFQQ(rq), *new_bfqq;
- assert_spin_locked(bfqd->queue->queue_lock);
+ assert_spin_locked(&bfqd->lock);
+
+ bfq_log_bfqq(bfqd, bfqq, "__insert_req: rq %p bfqq %p", rq, bfqq);
/*
* An unplug may trigger a requeue of a request from the device
@@ -4303,8 +4403,14 @@ static void bfq_insert_request(struct request_queue *q, struct request *rq)
* Release the request's reference to the old bfqq
* and make sure one is taken to the shared queue.
*/
- new_bfqq->allocated[rq_data_dir(rq)]++;
- bfqq->allocated[rq_data_dir(rq)]--;
+ new_bfqq->allocated++;
+ bfqq->allocated--;
+ bfq_log_bfqq(bfqd, bfqq,
+ "insert_request: new allocated %d", bfqq->allocated);
+ bfq_log_bfqq(bfqd, new_bfqq,
+ "insert_request: new_bfqq new allocated %d",
+ bfqq->allocated);
+
new_bfqq->ref++;
bfq_clear_bfqq_just_created(bfqq);
bfq_put_queue(bfqq);
@@ -4324,6 +4430,55 @@ static void bfq_insert_request(struct request_queue *q, struct request *rq)
bfq_rq_enqueued(bfqd, bfqq, rq);
}
+static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
+ bool at_head)
+{
+ struct request_queue *q = hctx->queue;
+ struct bfq_data *bfqd = q->elevator->elevator_data;
+
+ spin_lock_irq(&bfqd->lock);
+ if (blk_mq_sched_try_insert_merge(q, rq))
+ goto done;
+ spin_unlock_irq(&bfqd->lock);
+
+ blk_mq_sched_request_inserted(rq);
+
+ spin_lock_irq(&bfqd->lock);
+ if (at_head || blk_rq_is_passthrough(rq)) {
+ struct bfq_queue *bfqq = RQ_BFQQ(rq);
+
+ if (at_head)
+ list_add(&rq->queuelist, &bfqd->dispatch);
+ else
+ list_add_tail(&rq->queuelist, &bfqd->dispatch);
+
+ if (bfqq)
+ bfqq->dispatched++;
+ } else {
+ __bfq_insert_request(bfqd, rq);
+
+ if (rq_mergeable(rq)) {
+ elv_rqhash_add(q, rq);
+ if (!q->last_merge)
+ q->last_merge = rq;
+ }
+ }
+done:
+ spin_unlock_irq(&bfqd->lock);
+}
+
+static void bfq_insert_requests(struct blk_mq_hw_ctx *hctx,
+ struct list_head *list, bool at_head)
+{
+ while (!list_empty(list)) {
+ struct request *rq;
+
+ rq = list_first_entry(list, struct request, queuelist);
+ list_del_init(&rq->queuelist);
+ bfq_insert_request(hctx, rq, at_head);
+ }
+}
+
static void bfq_update_hw_tag(struct bfq_data *bfqd)
{
bfqd->max_rq_in_driver = max_t(int, bfqd->max_rq_in_driver,
@@ -4349,27 +4504,21 @@ static void bfq_update_hw_tag(struct bfq_data *bfqd)
bfqd->hw_tag_samples = 0;
}
-static void bfq_completed_request(struct request_queue *q, struct request *rq)
+static void bfq_completed_request(struct bfq_queue *bfqq, struct bfq_data *bfqd)
{
- struct bfq_queue *bfqq = RQ_BFQQ(rq);
- struct bfq_data *bfqd = bfqq->bfqd;
u64 now_ns;
u32 delta_us;
- bfq_log_bfqq(bfqd, bfqq, "completed one req with %u sects left",
- blk_rq_sectors(rq));
-
- assert_spin_locked(bfqd->queue->queue_lock);
bfq_update_hw_tag(bfqd);
BUG_ON(!bfqd->rq_in_driver);
BUG_ON(!bfqq->dispatched);
bfqd->rq_in_driver--;
bfqq->dispatched--;
- bfqg_stats_update_completion(bfqq_group(bfqq),
- rq_start_time_ns(rq),
- rq_io_start_time_ns(rq),
- rq->cmd_flags);
+
+ bfq_log_bfqq(bfqd, bfqq,
+ "completed_requests: new disp %d, new rq_in_driver %d",
+ bfqq->dispatched, bfqd->rq_in_driver);
if (!bfqq->dispatched && !bfq_bfqq_busy(bfqq)) {
BUG_ON(!RB_EMPTY_ROOT(&bfqq->sort_list));
@@ -4395,7 +4544,8 @@ static void bfq_completed_request(struct request_queue *q, struct request *rq)
*/
delta_us = div_u64(now_ns - bfqd->last_completion, NSEC_PER_USEC);
- bfq_log(bfqd, "rq_completed: delta %uus/%luus max_size %u rate %llu/%llu",
+ bfq_log_bfqq(bfqd, bfqq,
+ "rq_completed: delta %uus/%luus max_size %u rate %llu/%llu",
delta_us, BFQ_MIN_TT/NSEC_PER_USEC, bfqd->last_rq_max_size,
(USEC_PER_SEC*
(u64)((bfqd->last_rq_max_size<<BFQ_RATE_SHIFT)/delta_us))
@@ -4445,7 +4595,7 @@ static void bfq_completed_request(struct request_queue *q, struct request *rq)
if (bfqd->in_service_queue == bfqq) {
if (bfqq->dispatched == 0 && bfq_bfqq_must_idle(bfqq)) {
bfq_arm_slice_timer(bfqd);
- goto out;
+ return;
} else if (bfq_may_expire_for_budg_timeout(bfqq))
bfq_bfqq_expire(bfqd, bfqq, false,
BFQ_BFQQ_BUDGET_TIMEOUT);
@@ -4455,68 +4605,81 @@ static void bfq_completed_request(struct request_queue *q, struct request *rq)
bfq_bfqq_expire(bfqd, bfqq, false,
BFQ_BFQQ_NO_MORE_REQUESTS);
}
-
- if (!bfqd->rq_in_driver)
- bfq_schedule_dispatch(bfqd);
-
-out:
- return;
}
-static int __bfq_may_queue(struct bfq_queue *bfqq)
+static void bfq_put_rq_priv_body(struct bfq_queue *bfqq)
{
- if (bfq_bfqq_wait_request(bfqq) && bfq_bfqq_must_alloc(bfqq)) {
- bfq_clear_bfqq_must_alloc(bfqq);
- return ELV_MQUEUE_MUST;
- }
+ bfq_log_bfqq(bfqq->bfqd, bfqq,
+ "put_request_body: allocated %d", bfqq->allocated);
+ BUG_ON(!bfqq->allocated);
+ bfqq->allocated--;
- return ELV_MQUEUE_MAY;
+ bfq_put_queue(bfqq);
}
-static int bfq_may_queue(struct request_queue *q, unsigned int op)
+static void bfq_put_rq_private(struct request_queue *q, struct request *rq)
{
- struct bfq_data *bfqd = q->elevator->elevator_data;
- struct task_struct *tsk = current;
- struct bfq_io_cq *bic;
struct bfq_queue *bfqq;
+ struct bfq_data *bfqd;
+ struct bfq_io_cq *bic;
- /*
- * Don't force setup of a queue from here, as a call to may_queue
- * does not necessarily imply that a request actually will be
- * queued. So just lookup a possibly existing queue, or return
- * 'may queue' if that fails.
- */
- bic = bfq_bic_lookup(bfqd, tsk->io_context);
- if (!bic)
- return ELV_MQUEUE_MAY;
+ BUG_ON(!rq);
+ bfqq = RQ_BFQQ(rq);
+ BUG_ON(!bfqq);
- bfqq = bic_to_bfqq(bic, op_is_sync(op));
- if (bfqq)
- return __bfq_may_queue(bfqq);
+ bic = RQ_BIC(rq);
+ BUG_ON(!bic);
- return ELV_MQUEUE_MAY;
-}
+ bfqd = bfqq->bfqd;
+ BUG_ON(!bfqd);
-/*
- * Queue lock held here.
- */
-static void bfq_put_request(struct request *rq)
-{
- struct bfq_queue *bfqq = RQ_BFQQ(rq);
+ BUG_ON(rq->rq_flags & RQF_QUEUED);
+ BUG_ON(!(rq->rq_flags & RQF_ELVPRIV));
- if (bfqq) {
- const int rw = rq_data_dir(rq);
+ bfq_log_bfqq(bfqd, bfqq,
+ "putting rq %p with %u sects left, STARTED %d",
+ rq, blk_rq_sectors(rq),
+ rq->rq_flags & RQF_STARTED);
- BUG_ON(!bfqq->allocated[rw]);
- bfqq->allocated[rw]--;
+ if (rq->rq_flags & RQF_STARTED)
+ bfqg_stats_update_completion(bfqq_group(bfqq),
+ rq_start_time_ns(rq),
+ rq_io_start_time_ns(rq),
+ rq->cmd_flags);
- rq->elv.priv[0] = NULL;
- rq->elv.priv[1] = NULL;
+ BUG_ON(blk_rq_sectors(rq) == 0 && !(rq->rq_flags & RQF_STARTED));
- bfq_log_bfqq(bfqq->bfqd, bfqq, "put_request %p, %d",
- bfqq, bfqq->ref);
- bfq_put_queue(bfqq);
+ if (likely(rq->rq_flags & RQF_STARTED)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&bfqd->lock, flags);
+
+ bfq_completed_request(bfqq, bfqd);
+ bfq_put_rq_priv_body(bfqq);
+
+ spin_unlock_irqrestore(&bfqd->lock, flags);
+ } else {
+ /*
+ * Request rq may be still/already in the scheduler,
+ * in which case we need to remove it. And we cannot
+ * defer such a check and removal, to avoid
+ * inconsistencies in the time interval from the end
+ * of this function to the start of the deferred work.
+ * Fortunately, this situation occurs only in process
+ * context, so taking the scheduler lock does not
+ * cause any deadlock, even if other locks are already
+ * (correctly) held by this process.
+ */
+ BUG_ON(in_interrupt());
+
+ assert_spin_locked(&bfqd->lock);
+ if (!RB_EMPTY_NODE(&rq->rb_node))
+ bfq_remove_request(q, rq);
+ bfq_put_rq_priv_body(bfqq);
}
+
+ rq->elv.priv[0] = NULL;
+ rq->elv.priv[1] = NULL;
}
/*
@@ -4548,18 +4711,17 @@ bfq_split_bfqq(struct bfq_io_cq *bic, struct bfq_queue *bfqq)
/*
* Allocate bfq data structures associated with this request.
*/
-static int bfq_set_request(struct request_queue *q, struct request *rq,
- struct bio *bio, gfp_t gfp_mask)
+static int bfq_get_rq_private(struct request_queue *q, struct request *rq,
+ struct bio *bio)
{
struct bfq_data *bfqd = q->elevator->elevator_data;
struct bfq_io_cq *bic = icq_to_bic(rq->elv.icq);
- const int rw = rq_data_dir(rq);
const int is_sync = rq_is_sync(rq);
struct bfq_queue *bfqq;
- unsigned long flags;
bool split = false;
- spin_lock_irqsave(q->queue_lock, flags);
+ spin_lock_irq(&bfqd->lock);
+
bfq_check_ioprio_change(bic, bio);
if (!bic)
@@ -4578,7 +4740,7 @@ static int bfq_set_request(struct request_queue *q, struct request *rq,
bic_set_bfqq(bic, bfqq, is_sync);
if (split && is_sync) {
bfq_log_bfqq(bfqd, bfqq,
- "set_request: was_in_list %d "
+ "get_request: was_in_list %d "
"was_in_large_burst %d "
"large burst in progress %d",
bic->was_in_burst_list,
@@ -4588,12 +4750,12 @@ static int bfq_set_request(struct request_queue *q, struct request *rq,
if ((bic->was_in_burst_list && bfqd->large_burst) ||
bic->saved_in_large_burst) {
bfq_log_bfqq(bfqd, bfqq,
- "set_request: marking in "
+ "get_request: marking in "
"large burst");
bfq_mark_bfqq_in_large_burst(bfqq);
} else {
bfq_log_bfqq(bfqd, bfqq,
- "set_request: clearing in "
+ "get_request: clearing in "
"large burst");
bfq_clear_bfqq_in_large_burst(bfqq);
if (bic->was_in_burst_list)
@@ -4618,9 +4780,12 @@ static int bfq_set_request(struct request_queue *q, struct request *rq,
}
}
- bfqq->allocated[rw]++;
+ bfqq->allocated++;
+ bfq_log_bfqq(bfqq->bfqd, bfqq,
+ "get_request: new allocated %d", bfqq->allocated);
+
bfqq->ref++;
- bfq_log_bfqq(bfqd, bfqq, "set_request: bfqq %p, %d", bfqq, bfqq->ref);
+ bfq_log_bfqq(bfqd, bfqq, "get_request: bfqq %p, %d", bfqq, bfqq->ref);
rq->elv.priv[0] = bic;
rq->elv.priv[1] = bfqq;
@@ -4647,26 +4812,55 @@ static int bfq_set_request(struct request_queue *q, struct request *rq,
if (unlikely(bfq_bfqq_just_created(bfqq)))
bfq_handle_burst(bfqd, bfqq);
- spin_unlock_irqrestore(q->queue_lock, flags);
+ spin_unlock_irq(&bfqd->lock);
return 0;
queue_fail:
- bfq_schedule_dispatch(bfqd);
- spin_unlock_irqrestore(q->queue_lock, flags);
+ spin_unlock_irq(&bfqd->lock);
return 1;
}
-static void bfq_kick_queue(struct work_struct *work)
+static void bfq_idle_slice_timer_body(struct bfq_queue *bfqq)
{
- struct bfq_data *bfqd =
- container_of(work, struct bfq_data, unplug_work);
- struct request_queue *q = bfqd->queue;
+ struct bfq_data *bfqd = bfqq->bfqd;
+ enum bfqq_expiration reason;
+ unsigned long flags;
+
+ BUG_ON(!bfqd);
+ spin_lock_irqsave(&bfqd->lock, flags);
+ bfq_log_bfqq(bfqd, bfqq, "handling slice_timer expiration");
+ bfq_clear_bfqq_wait_request(bfqq);
- spin_lock_irq(q->queue_lock);
- __blk_run_queue(q);
- spin_unlock_irq(q->queue_lock);
+ if (bfqq != bfqd->in_service_queue) {
+ spin_unlock_irqrestore(&bfqd->lock, flags);
+ return;
+ }
+
+ if (bfq_bfqq_budget_timeout(bfqq))
+ /*
+ * Also here the queue can be safely expired
+ * for budget timeout without wasting
+ * guarantees
+ */
+ reason = BFQ_BFQQ_BUDGET_TIMEOUT;
+ else if (bfqq->queued[0] == 0 && bfqq->queued[1] == 0)
+ /*
+ * The queue may not be empty upon timer expiration,
+ * because we may not disable the timer when the
+ * first request of the in-service queue arrives
+ * during disk idling.
+ */
+ reason = BFQ_BFQQ_TOO_IDLE;
+ else
+ goto schedule_dispatch;
+
+ bfq_bfqq_expire(bfqd, bfqq, true, reason);
+
+schedule_dispatch:
+ spin_unlock_irqrestore(&bfqd->lock, flags);
+ bfq_schedule_dispatch(bfqd);
}
/*
@@ -4677,59 +4871,24 @@ static enum hrtimer_restart bfq_idle_slice_timer(struct hrtimer *timer)
{
struct bfq_data *bfqd = container_of(timer, struct bfq_data,
idle_slice_timer);
- struct bfq_queue *bfqq;
- unsigned long flags;
- enum bfqq_expiration reason;
+ struct bfq_queue *bfqq = bfqd->in_service_queue;
- spin_lock_irqsave(bfqd->queue->queue_lock, flags);
+ bfq_log(bfqd, "slice_timer expired");
- bfqq = bfqd->in_service_queue;
/*
* Theoretical race here: the in-service queue can be NULL or
- * different from the queue that was idling if the timer handler
- * spins on the queue_lock and a new request arrives for the
- * current queue and there is a full dispatch cycle that changes
- * the in-service queue. This can hardly happen, but in the worst
- * case we just expire a queue too early.
+ * different from the queue that was idling if a new request
+ * arrives for the current queue and there is a full dispatch
+ * cycle that changes the in-service queue. This can hardly
+ * happen, but in the worst case we just expire a queue too
+ * early.
*/
- if (bfqq) {
- bfq_log_bfqq(bfqd, bfqq, "slice_timer expired");
- bfq_clear_bfqq_wait_request(bfqq);
-
- if (bfq_bfqq_budget_timeout(bfqq))
- /*
- * Also here the queue can be safely expired
- * for budget timeout without wasting
- * guarantees
- */
- reason = BFQ_BFQQ_BUDGET_TIMEOUT;
- else if (bfqq->queued[0] == 0 && bfqq->queued[1] == 0)
- /*
- * The queue may not be empty upon timer expiration,
- * because we may not disable the timer when the
- * first request of the in-service queue arrives
- * during disk idling.
- */
- reason = BFQ_BFQQ_TOO_IDLE;
- else
- goto schedule_dispatch;
-
- bfq_bfqq_expire(bfqd, bfqq, true, reason);
- }
-
-schedule_dispatch:
- bfq_schedule_dispatch(bfqd);
+ if (bfqq)
+ bfq_idle_slice_timer_body(bfqq);
- spin_unlock_irqrestore(bfqd->queue->queue_lock, flags);
return HRTIMER_NORESTART;
}
-static void bfq_shutdown_timer_wq(struct bfq_data *bfqd)
-{
- hrtimer_cancel(&bfqd->idle_slice_timer);
- cancel_work_sync(&bfqd->unplug_work);
-}
-
static void __bfq_put_async_bfqq(struct bfq_data *bfqd,
struct bfq_queue **bfqq_ptr)
{
@@ -4766,30 +4925,44 @@ static void bfq_put_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg)
static void bfq_exit_queue(struct elevator_queue *e)
{
struct bfq_data *bfqd = e->elevator_data;
- struct request_queue *q = bfqd->queue;
struct bfq_queue *bfqq, *n;
- bfq_shutdown_timer_wq(bfqd);
+ bfq_log(bfqd, "exit_queue: starting ...");
- spin_lock_irq(q->queue_lock);
+ hrtimer_cancel(&bfqd->idle_slice_timer);
BUG_ON(bfqd->in_service_queue);
- list_for_each_entry_safe(bfqq, n, &bfqd->idle_list, bfqq_list)
- bfq_deactivate_bfqq(bfqd, bfqq, false, false);
+ BUG_ON(!list_empty(&bfqd->active_list));
- spin_unlock_irq(q->queue_lock);
+ list_for_each_entry_safe(bfqq, n, &bfqd->idle_list, bfqq_list) {
+ if (bfqq->bic) /* bfqqs without bic are handled below */
+ cancel_work_sync(&bfqq->bic->exit_icq_work);
+ }
- bfq_shutdown_timer_wq(bfqd);
+ spin_lock_irq(&bfqd->lock);
+ list_for_each_entry_safe(bfqq, n, &bfqd->idle_list, bfqq_list) {
+ bfq_deactivate_bfqq(bfqd, bfqq, false, false);
+ /*
+ * Make sure that deferred exit_icq_work completes
+ * without errors for bfq_queues without bic
+ */
+ if (!bfqq->bic)
+ bfqq->bfqd = NULL;
+ }
+ spin_unlock_irq(&bfqd->lock);
+
+ hrtimer_cancel(&bfqd->idle_slice_timer);
BUG_ON(hrtimer_active(&bfqd->idle_slice_timer));
#ifdef BFQ_GROUP_IOSCHED_ENABLED
- blkcg_deactivate_policy(q, &blkcg_policy_bfq);
+ blkcg_deactivate_policy(bfqd->queue, &blkcg_policy_bfq);
#else
bfq_put_async_queues(bfqd, bfqd->root_group);
kfree(bfqd->root_group);
#endif
+ bfq_log(bfqd, "exit_queue: finished ...");
kfree(bfqd);
}
@@ -4848,10 +5021,6 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
bfqd->queue = q;
- spin_lock_irq(q->queue_lock);
- q->elevator = eq;
- spin_unlock_irq(q->queue_lock);
-
bfqd->root_group = bfq_create_group_hierarchy(bfqd, q->node);
if (!bfqd->root_group)
goto out_free;
@@ -4865,8 +5034,6 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
bfqd->queue_weights_tree = RB_ROOT;
bfqd->group_weights_tree = RB_ROOT;
- INIT_WORK(&bfqd->unplug_work, bfq_kick_queue);
-
INIT_LIST_HEAD(&bfqd->active_list);
INIT_LIST_HEAD(&bfqd->idle_list);
INIT_HLIST_HEAD(&bfqd->burst_list);
@@ -4915,6 +5082,11 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
bfqd->peak_rate = R_fast[blk_queue_nonrot(bfqd->queue)] * 2 / 3;
bfqd->device_speed = BFQ_BFQD_FAST;
+ spin_lock_init(&bfqd->lock);
+ INIT_LIST_HEAD(&bfqd->dispatch);
+
+ q->elevator = eq;
+
return 0;
out_free:
@@ -4971,7 +5143,7 @@ static ssize_t bfq_weights_show(struct elevator_queue *e, char *page)
num_char += sprintf(page + num_char, "Tot reqs queued %d\n\n",
bfqd->queued);
- spin_lock_irq(bfqd->queue->queue_lock);
+ spin_lock_irq(&bfqd->lock);
num_char += sprintf(page + num_char, "Active:\n");
list_for_each_entry(bfqq, &bfqd->active_list, bfqq_list) {
@@ -5000,7 +5172,7 @@ static ssize_t bfq_weights_show(struct elevator_queue *e, char *page)
jiffies_to_msecs(bfqq->wr_cur_max_time));
}
- spin_unlock_irq(bfqd->queue->queue_lock);
+ spin_unlock_irq(&bfqd->lock);
return num_char;
}
@@ -5208,35 +5380,31 @@ static struct elv_fs_entry bfq_attrs[] = {
__ATTR_NULL
};
-static struct elevator_type iosched_bfq = {
- .ops.sq = {
- .elevator_merge_fn = bfq_merge,
- .elevator_merged_fn = bfq_merged_request,
- .elevator_merge_req_fn = bfq_merged_requests,
-#ifdef BFQ_GROUP_IOSCHED_ENABLED
- .elevator_bio_merged_fn = bfq_bio_merged,
-#endif
- .elevator_allow_bio_merge_fn = bfq_allow_bio_merge,
- .elevator_allow_rq_merge_fn = bfq_allow_rq_merge,
- .elevator_dispatch_fn = bfq_dispatch_requests,
- .elevator_add_req_fn = bfq_insert_request,
- .elevator_activate_req_fn = bfq_activate_request,
- .elevator_deactivate_req_fn = bfq_deactivate_request,
- .elevator_completed_req_fn = bfq_completed_request,
- .elevator_former_req_fn = elv_rb_former_request,
- .elevator_latter_req_fn = elv_rb_latter_request,
- .elevator_init_icq_fn = bfq_init_icq,
- .elevator_exit_icq_fn = bfq_exit_icq,
- .elevator_set_req_fn = bfq_set_request,
- .elevator_put_req_fn = bfq_put_request,
- .elevator_may_queue_fn = bfq_may_queue,
- .elevator_init_fn = bfq_init_queue,
- .elevator_exit_fn = bfq_exit_queue,
+static struct elevator_type iosched_bfq_mq = {
+ .ops.mq = {
+ .get_rq_priv = bfq_get_rq_private,
+ .put_rq_priv = bfq_put_rq_private,
+ .init_icq = bfq_init_icq,
+ .exit_icq = bfq_exit_icq,
+ .insert_requests = bfq_insert_requests,
+ .dispatch_request = bfq_dispatch_request,
+ .next_request = elv_rb_latter_request,
+ .former_request = elv_rb_former_request,
+ .allow_merge = bfq_allow_bio_merge,
+ .bio_merge = bfq_bio_merge,
+ .request_merge = bfq_request_merge,
+ .requests_merged = bfq_requests_merged,
+ .request_merged = bfq_request_merged,
+ .has_work = bfq_has_work,
+ .init_sched = bfq_init_queue,
+ .exit_sched = bfq_exit_queue,
},
+
+ .uses_mq = true,
.icq_size = sizeof(struct bfq_io_cq),
.icq_align = __alignof__(struct bfq_io_cq),
.elevator_attrs = bfq_attrs,
- .elevator_name = "bfq",
+ .elevator_name = "bfq-mq",
.elevator_owner = THIS_MODULE,
};
@@ -5261,7 +5429,7 @@ static struct blkcg_policy blkcg_policy_bfq = {
static int __init bfq_init(void)
{
int ret;
- char msg[60] = "BFQ I/O-scheduler: v8r8-rc2";
+ char msg[60] = "BFQ-MQ I/O-scheduler: v8r8-rc2";
#ifdef BFQ_GROUP_IOSCHED_ENABLED
ret = blkcg_policy_register(&blkcg_policy_bfq);
@@ -5306,7 +5474,7 @@ static int __init bfq_init(void)
device_speed_thresh[0] = (4 * R_slow[0]) / 3;
device_speed_thresh[1] = (4 * R_slow[1]) / 3;
- ret = elv_register(&iosched_bfq);
+ ret = elv_register(&iosched_bfq_mq);
if (ret)
goto err_pol_unreg;
@@ -5326,8 +5494,8 @@ static int __init bfq_init(void)
static void __exit bfq_exit(void)
{
- elv_unregister(&iosched_bfq);
-#ifdef BFQ_GROUP_IOSCHED_ENABLED
+ elv_unregister(&iosched_bfq_mq);
+#ifdef CONFIG_BFQ_GROUP_ENABLED
blkcg_policy_unregister(&blkcg_policy_bfq);
#endif
bfq_slab_kill();
@@ -5336,5 +5504,6 @@ static void __exit bfq_exit(void)
module_init(bfq_init);
module_exit(bfq_exit);
-MODULE_AUTHOR("Arianna Avanzini, Fabio Checconi, Paolo Valente");
+MODULE_AUTHOR("Paolo Valente");
MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MQ Budget Fair IO scheduler");
diff --git a/block/bfq-mq.h b/block/bfq-mq.h
index c6acee2..6e1c0d8 100644
--- a/block/bfq-mq.h
+++ b/block/bfq-mq.h
@@ -1,5 +1,5 @@
/*
- * BFQ v8r8-rc2 for 4.10.0: data structures and common functions prototypes.
+ * BFQ-MQ v8r8-rc2 for 4.10.0: data structures and common functions prototypes.
*
* Based on ideas and code from CFQ:
* Copyright (C) 2003 Jens Axboe <axboe@kernel.dk>
@@ -21,15 +21,8 @@
#include <linux/rbtree.h>
#include <linux/blk-cgroup.h>
-/*
- * Define an alternative macro to compile cgroups support. This is one
- * of the steps needed to let bfq-mq share the files bfq-sched.c and
- * bfq-cgroup.c with bfq. For bfq-mq, the macro
- * BFQ_GROUP_IOSCHED_ENABLED will be defined as a function of whether
- * the configuration option CONFIG_BFQ_MQ_GROUP_IOSCHED, and not
- * CONFIG_BFQ_GROUP_IOSCHED, is defined.
- */
-#ifdef CONFIG_BFQ_GROUP_IOSCHED
+/* see comments on CONFIG_BFQ_GROUP_IOSCHED in bfq.h */
+#ifdef CONFIG_BFQ_MQ_GROUP_IOSCHED
#define BFQ_GROUP_IOSCHED_ENABLED
#endif
@@ -248,8 +241,8 @@ struct bfq_queue {
struct request *next_rq;
/* number of sync and async requests queued */
int queued[2];
- /* number of sync and async requests currently allocated */
- int allocated[2];
+ /* number of requests currently allocated */
+ int allocated;
/* number of pending metadata requests */
int meta_pending;
/* fifo list of requests in sort_list */
@@ -334,6 +327,8 @@ struct bfq_queue {
unsigned long wr_start_at_switch_to_srt;
unsigned long split_time; /* time of last split */
+
+ spinlock_t lock;
};
/**
@@ -350,6 +345,9 @@ struct bfq_io_cq {
uint64_t blkcg_serial_nr; /* the current blkcg serial */
#endif
+ /* delayed work to exec the body of the the exit_icq handler */
+ struct work_struct exit_icq_work;
+
/*
* Snapshot of the idle window before merging; taken to
* remember this value while the queue is merged, so as to be
@@ -391,11 +389,13 @@ enum bfq_device_speed {
/**
* struct bfq_data - per-device data structure.
*
- * All the fields are protected by the @queue lock.
+ * All the fields are protected by @lock.
*/
struct bfq_data {
- /* request queue for the device */
+ /* device request queue */
struct request_queue *queue;
+ /* dispatch queue */
+ struct list_head dispatch;
/* root bfq_group for the device */
struct bfq_group *root_group;
@@ -449,8 +449,6 @@ struct bfq_data {
* the queue in service.
*/
struct hrtimer idle_slice_timer;
- /* delayed work to restart dispatching on the request queue */
- struct work_struct unplug_work;
/* bfq_queue in service */
struct bfq_queue *in_service_queue;
@@ -603,6 +601,8 @@ struct bfq_data {
/* fallback dummy bfqq for extreme OOM conditions */
struct bfq_queue oom_bfqq;
+
+ spinlock_t lock;
};
enum bfqq_state_flags {
@@ -613,7 +613,6 @@ enum bfqq_state_flags {
* waiting for a request
* without idling the device
*/
- BFQ_BFQQ_FLAG_must_alloc, /* must be allowed rq alloc */
BFQ_BFQQ_FLAG_fifo_expire, /* FIFO checked in this slice */
BFQ_BFQQ_FLAG_idle_window, /* slice idling enabled */
BFQ_BFQQ_FLAG_sync, /* synchronous queue */
@@ -652,7 +651,6 @@ BFQ_BFQQ_FNS(just_created);
BFQ_BFQQ_FNS(busy);
BFQ_BFQQ_FNS(wait_request);
BFQ_BFQQ_FNS(non_blocking_wait_rq);
-BFQ_BFQQ_FNS(must_alloc);
BFQ_BFQQ_FNS(fifo_expire);
BFQ_BFQQ_FNS(idle_window);
BFQ_BFQQ_FNS(sync);
@@ -672,7 +670,6 @@ static struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);
#define bfq_log_bfqq(bfqd, bfqq, fmt, args...) do { \
char __pbuf[128]; \
\
- assert_spin_locked((bfqd)->queue->queue_lock); \
blkg_path(bfqg_to_blkg(bfqq_group(bfqq)), __pbuf, sizeof(__pbuf)); \
pr_crit("bfq%d%c %s " fmt "\n", \
(bfqq)->pid, \
@@ -708,7 +705,6 @@ static struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);
#define bfq_log_bfqq(bfqd, bfqq, fmt, args...) do { \
char __pbuf[128]; \
\
- assert_spin_locked((bfqd)->queue->queue_lock); \
blkg_path(bfqg_to_blkg(bfqq_group(bfqq)), __pbuf, sizeof(__pbuf)); \
blk_add_trace_msg((bfqd)->queue, "bfq%d%c %s " fmt, \
(bfqq)->pid, \
@@ -935,7 +931,6 @@ static struct bfq_group *bfq_bfqq_to_bfqg(struct bfq_queue *bfqq)
static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio);
static void bfq_put_queue(struct bfq_queue *bfqq);
-static void bfq_dispatch_insert(struct request_queue *q, struct request *rq);
static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd,
struct bio *bio, bool is_sync,
struct bfq_io_cq *bic);
--
2.10.0
^ permalink raw reply related
* [WIP PATCHSET 3/4] Embed bfq-ioc.c and add locking on request queue
From: Paolo Valente @ 2017-02-07 17:24 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij, broonie,
Paolo Valente
In-Reply-To: <20170207172446.4528-1-paolo.valente@linaro.org>
The version of bfq-ioc.c for bfq-iosched.c is not correct any more for
bfq-mq, because, in bfq-mq, the request queue lock is not being held
when bfq_bic_lookup is invoked. That function must then take that look
on its own. This commit removes the inclusion of bfq-ioc.c, copies the
content of bfq-ioc.c into bfq-mq-iosched.c, and adds the grabbing of
the lock.
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
block/bfq-mq-iosched.c | 39 ++++++++++++++++++++++++++++++++++++---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/block/bfq-mq-iosched.c b/block/bfq-mq-iosched.c
index 7e3f429..a8679de 100644
--- a/block/bfq-mq-iosched.c
+++ b/block/bfq-mq-iosched.c
@@ -190,7 +190,39 @@ static int device_speed_thresh[2];
static void bfq_schedule_dispatch(struct bfq_data *bfqd);
-#include "bfq-ioc.c"
+/**
+ * icq_to_bic - convert iocontext queue structure to bfq_io_cq.
+ * @icq: the iocontext queue.
+ */
+static struct bfq_io_cq *icq_to_bic(struct io_cq *icq)
+{
+ /* bic->icq is the first member, %NULL will convert to %NULL */
+ return container_of(icq, struct bfq_io_cq, icq);
+}
+
+/**
+ * bfq_bic_lookup - search into @ioc a bic associated to @bfqd.
+ * @bfqd: the lookup key.
+ * @ioc: the io_context of the process doing I/O.
+ * @q: the request queue.
+ */
+static struct bfq_io_cq *bfq_bic_lookup(struct bfq_data *bfqd,
+ struct io_context *ioc,
+ struct request_queue *q)
+{
+ if (ioc) {
+ struct bfq_io_cq *icq;
+
+ spin_lock_irq(q->queue_lock);
+ icq = icq_to_bic(ioc_lookup_icq(ioc, q));
+ spin_unlock_irq(q->queue_lock);
+
+ return icq;
+ }
+
+ return NULL;
+}
+
#include "bfq-sched.c"
#include "bfq-cgroup.c"
@@ -1480,13 +1512,14 @@ static void bfq_add_request(struct request *rq)
}
static struct request *bfq_find_rq_fmerge(struct bfq_data *bfqd,
- struct bio *bio)
+ struct bio *bio,
+ struct request_queue *q)
{
struct task_struct *tsk = current;
struct bfq_io_cq *bic;
struct bfq_queue *bfqq;
- bic = bfq_bic_lookup(bfqd, tsk->io_context);
+ bic = bfq_bic_lookup(bfqd, tsk->io_context, q);
if (!bic)
return NULL;
--
2.10.0
^ permalink raw reply related
* [WIP PATCHSET 2/4] Move thinktime from bic to bfqq
From: Paolo Valente @ 2017-02-07 17:24 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij, broonie,
Paolo Valente
In-Reply-To: <20170207172446.4528-1-paolo.valente@linaro.org>
Prep change to make it possible to protect this field with a
scheduler lock.
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
block/bfq-mq-iosched.c | 28 ++++++++++++++--------------
block/bfq-mq.h | 30 ++++++++++++++++--------------
2 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/block/bfq-mq-iosched.c b/block/bfq-mq-iosched.c
index 0a7da4e..7e3f429 100644
--- a/block/bfq-mq-iosched.c
+++ b/block/bfq-mq-iosched.c
@@ -670,6 +670,7 @@ bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
else
bfq_clear_bfqq_IO_bound(bfqq);
+ bfqq->ttime = bic->saved_ttime;
bfqq->wr_coeff = bic->saved_wr_coeff;
bfqq->wr_start_at_switch_to_srt = bic->saved_wr_start_at_switch_to_srt;
BUG_ON(time_is_after_jiffies(bfqq->wr_start_at_switch_to_srt));
@@ -1247,7 +1248,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
* details on the usage of the next variable.
*/
arrived_in_time = ktime_get_ns() <=
- RQ_BIC(rq)->ttime.last_end_request +
+ bfqq->ttime.last_end_request +
bfqd->bfq_slice_idle * 3;
bfq_log_bfqq(bfqd, bfqq,
@@ -2003,6 +2004,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
if (!bic)
return;
+ bic->saved_ttime = bfqq->ttime;
bic->saved_idle_window = bfq_bfqq_idle_window(bfqq);
bic->saved_IO_bound = bfq_bfqq_IO_bound(bfqq);
bic->saved_in_large_burst = bfq_bfqq_in_large_burst(bfqq);
@@ -3870,11 +3872,6 @@ static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq)
bfq_put_queue(bfqq);
}
-static void bfq_init_icq(struct io_cq *icq)
-{
- icq_to_bic(icq)->ttime.last_end_request = ktime_get_ns() - (1ULL<<32);
-}
-
static void bfq_exit_icq(struct io_cq *icq)
{
struct bfq_io_cq *bic = icq_to_bic(icq);
@@ -4000,6 +3997,9 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
bfq_mark_bfqq_just_created(bfqq);
} else
bfq_clear_bfqq_sync(bfqq);
+
+ bfqq->ttime.last_end_request = ktime_get_ns() - (1ULL<<32);
+
bfq_mark_bfqq_IO_bound(bfqq);
/* Tentative initial value to trade off between thr and lat */
@@ -4107,14 +4107,14 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd,
}
static void bfq_update_io_thinktime(struct bfq_data *bfqd,
- struct bfq_io_cq *bic)
+ struct bfq_queue *bfqq)
{
- struct bfq_ttime *ttime = &bic->ttime;
- u64 elapsed = ktime_get_ns() - bic->ttime.last_end_request;
+ struct bfq_ttime *ttime = &bfqq->ttime;
+ u64 elapsed = ktime_get_ns() - bfqq->ttime.last_end_request;
elapsed = min_t(u64, elapsed, 2 * bfqd->bfq_slice_idle);
- ttime->ttime_samples = (7*bic->ttime.ttime_samples + 256) / 8;
+ ttime->ttime_samples = (7*bfqq->ttime.ttime_samples + 256) / 8;
ttime->ttime_total = div_u64(7*ttime->ttime_total + 256*elapsed, 8);
ttime->ttime_mean = div64_ul(ttime->ttime_total + 128,
ttime->ttime_samples);
@@ -4157,8 +4157,8 @@ static void bfq_update_idle_window(struct bfq_data *bfqd,
(bfqd->hw_tag && BFQQ_SEEKY(bfqq) &&
bfqq->wr_coeff == 1))
enable_idle = 0;
- else if (bfq_sample_valid(bic->ttime.ttime_samples)) {
- if (bic->ttime.ttime_mean > bfqd->bfq_slice_idle &&
+ else if (bfq_sample_valid(bfqq->ttime.ttime_samples)) {
+ if (bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle &&
bfqq->wr_coeff == 1)
enable_idle = 0;
else
@@ -4185,7 +4185,7 @@ static void bfq_rq_enqueued(struct bfq_data *bfqd, struct bfq_queue *bfqq,
if (rq->cmd_flags & REQ_META)
bfqq->meta_pending++;
- bfq_update_io_thinktime(bfqd, bic);
+ bfq_update_io_thinktime(bfqd, bfqq);
bfq_update_io_seektime(bfqd, bfqq, rq);
if (bfqq->entity.service > bfq_max_budget(bfqd) / 8 ||
!BFQQ_SEEKY(bfqq))
@@ -4354,7 +4354,7 @@ static void bfq_completed_request(struct request_queue *q, struct request *rq)
now_ns = ktime_get_ns();
- RQ_BIC(rq)->ttime.last_end_request = now_ns;
+ bfqq->ttime.last_end_request = now_ns;
/*
* Using us instead of ns, to get a reasonable precision in
diff --git a/block/bfq-mq.h b/block/bfq-mq.h
index f28feb6..c6acee2 100644
--- a/block/bfq-mq.h
+++ b/block/bfq-mq.h
@@ -199,6 +199,18 @@ struct bfq_entity {
struct bfq_group;
/**
+ * struct bfq_ttime - per process thinktime stats.
+ */
+struct bfq_ttime {
+ u64 last_end_request; /* completion time of last request */
+
+ u64 ttime_total; /* total process thinktime */
+ unsigned long ttime_samples; /* number of thinktime samples */
+ u64 ttime_mean; /* average process thinktime */
+
+};
+
+/**
* struct bfq_queue - leaf schedulable entity.
*
* A bfq_queue is a leaf request queue; it can be associated with an
@@ -259,6 +271,9 @@ struct bfq_queue {
/* node for active/idle bfqq list inside parent bfqd */
struct list_head bfqq_list;
+ /* associated @bfq_ttime struct */
+ struct bfq_ttime ttime;
+
/* bit vector: a 1 for each seeky requests in history */
u32 seek_history;
@@ -322,18 +337,6 @@ struct bfq_queue {
};
/**
- * struct bfq_ttime - per process thinktime stats.
- */
-struct bfq_ttime {
- u64 last_end_request; /* completion time of last request */
-
- u64 ttime_total; /* total process thinktime */
- unsigned long ttime_samples; /* number of thinktime samples */
- u64 ttime_mean; /* average process thinktime */
-
-};
-
-/**
* struct bfq_io_cq - per (request_queue, io_context) structure.
*/
struct bfq_io_cq {
@@ -341,8 +344,6 @@ struct bfq_io_cq {
struct io_cq icq; /* must be the first member */
/* array of two process queues, the sync and the async */
struct bfq_queue *bfqq[2];
- /* associated @bfq_ttime struct */
- struct bfq_ttime ttime;
/* per (request_queue, blkcg) ioprio */
int ioprio;
#ifdef BFQ_GROUP_IOSCHED_ENABLED
@@ -379,6 +380,7 @@ struct bfq_io_cq {
unsigned long saved_last_wr_start_finish;
unsigned long saved_wr_start_at_switch_to_srt;
unsigned int saved_wr_cur_max_time;
+ struct bfq_ttime saved_ttime;
};
enum bfq_device_speed {
--
2.10.0
^ permalink raw reply related
* [WIP PATCHSET 1/4] blk-mq: pass bio to blk_mq_sched_get_rq_priv
From: Paolo Valente @ 2017-02-07 17:24 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij, broonie,
Paolo Valente
In-Reply-To: <20170207172446.4528-1-paolo.valente@linaro.org>
bio is used in bfq-mq's get_rq_priv, to get the request group. We could
pass directly the group here, but I thought that passing the bio was
more general, giving the possibility to get other pieces of information
if needed.
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
block/blk-mq-sched.c | 8 +++++---
block/blk-mq-sched.h | 5 +++--
include/linux/elevator.h | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index ee455e7..314a8ed 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -68,7 +68,9 @@ int blk_mq_sched_init_hctx_data(struct request_queue *q, size_t size,
EXPORT_SYMBOL_GPL(blk_mq_sched_init_hctx_data);
static void __blk_mq_sched_assign_ioc(struct request_queue *q,
- struct request *rq, struct io_context *ioc)
+ struct request *rq,
+ struct bio *bio,
+ struct io_context *ioc)
{
struct io_cq *icq;
@@ -83,7 +85,7 @@ static void __blk_mq_sched_assign_ioc(struct request_queue *q,
}
rq->elv.icq = icq;
- if (!blk_mq_sched_get_rq_priv(q, rq)) {
+ if (!blk_mq_sched_get_rq_priv(q, rq, bio)) {
rq->rq_flags |= RQF_ELVPRIV;
get_io_context(icq->ioc);
return;
@@ -99,7 +101,7 @@ static void blk_mq_sched_assign_ioc(struct request_queue *q,
ioc = rq_ioc(bio);
if (ioc)
- __blk_mq_sched_assign_ioc(q, rq, ioc);
+ __blk_mq_sched_assign_ioc(q, rq, bio, ioc);
}
struct request *blk_mq_sched_get_request(struct request_queue *q,
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 5954859..7b5f3b9 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -49,12 +49,13 @@ blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio)
}
static inline int blk_mq_sched_get_rq_priv(struct request_queue *q,
- struct request *rq)
+ struct request *rq,
+ struct bio *bio)
{
struct elevator_queue *e = q->elevator;
if (e && e->type->ops.mq.get_rq_priv)
- return e->type->ops.mq.get_rq_priv(q, rq);
+ return e->type->ops.mq.get_rq_priv(q, rq, bio);
return 0;
}
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index b5825c4..ce14ede 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -99,7 +99,7 @@ struct elevator_mq_ops {
void (*requeue_request)(struct request *);
struct request *(*former_request)(struct request_queue *, struct request *);
struct request *(*next_request)(struct request_queue *, struct request *);
- int (*get_rq_priv)(struct request_queue *, struct request *);
+ int (*get_rq_priv)(struct request_queue *, struct request *, struct bio *);
void (*put_rq_priv)(struct request_queue *, struct request *);
void (*init_icq)(struct io_cq *);
void (*exit_icq)(struct io_cq *);
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 0/4 v2] BDI lifetime fix
From: Tejun Heo @ 2017-02-07 17:21 UTC (permalink / raw)
To: Jan Kara
Cc: Thiago Jung Bauermann, Jens Axboe, linux-block, Christoph Hellwig,
Dan Williams, Laurent Dufour
In-Reply-To: <20170207123331.GD13524@quack2.suse.cz>
Hello,
On Tue, Feb 07, 2017 at 01:33:31PM +0100, Jan Kara wrote:
> > We can see above that inode->i_wb is in r31, and the machine crashed at
> > 0xc0000000003799a0 so it was trying to dereference wb and crashed.
> > r31 is NULL in the crash information.
>
> Thanks for report and the analysis. After some looking into the code I see
> where the problem is. Writeback code assumes inode->i_wb can never become
> invalid once it is set however we still call inode_detach_wb() from
> __blkdev_put(). So in a way this is a different problem but closely
> related.
Heh, it feels like we're chasing our own tails.
> It seems to me that instead of calling inode_detach_wb() in __blkdev_put()
> we may just switch blkdev inode to bdi->wb (it is now guaranteed to stay
> around). That way bdi_unregister() can complete (destroying all writeback
> structures except for bdi->wb) while block device inode can still live with
> a valid i_wb structure.
So, the problem there would be synchronizing get_wb against the
transition. We can do that and inode_switch_wbs_work_fn() already
does it but it is a bit nasty.
I'm getting a bit confused here, so the reason we added
inode_detach_wb() in __blkdev_put() was because the root wb might go
away because it's embedded in the bdi which is embedded in the
request_queue which is put and may be released by put_disk().
Are you saying that we changed the behavior so that bdi->wb stays
around? If so, we can just remove the inode_detach_wb() call, no?
Thanks.
--
tejun
^ permalink raw reply
* [PATCH 4/4] nvme: support ranged discard requests
From: Christoph Hellwig @ 2017-02-07 16:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170207164658.32449-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index efe8ec300126..ae9254fcd9c5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -238,26 +238,36 @@ static inline void nvme_setup_flush(struct nvme_ns *ns,
static inline int nvme_setup_discard(struct nvme_ns *ns, struct request *req,
struct nvme_command *cmnd)
{
+ unsigned short segments = blk_rq_nr_discard_segments(req), n = 0;
struct nvme_dsm_range *range;
- unsigned int nr_bytes = blk_rq_bytes(req);
+ struct bio *bio;
- range = kmalloc(sizeof(*range), GFP_ATOMIC);
+ range = kmalloc_array(segments, sizeof(*range), GFP_ATOMIC);
if (!range)
return BLK_MQ_RQ_QUEUE_BUSY;
- range->cattr = cpu_to_le32(0);
- range->nlb = cpu_to_le32(nr_bytes >> ns->lba_shift);
- range->slba = cpu_to_le64(nvme_block_nr(ns, blk_rq_pos(req)));
+ __rq_for_each_bio(bio, req) {
+ u64 slba = nvme_block_nr(ns, bio->bi_iter.bi_sector);
+ u32 nlb = bio->bi_iter.bi_size >> ns->lba_shift;
+
+ range[n].cattr = cpu_to_le32(0);
+ range[n].nlb = cpu_to_le32(nlb);
+ range[n].slba = cpu_to_le64(slba);
+ n++;
+ }
+
+ if (WARN_ON_ONCE(n != segments))
+ return BLK_MQ_RQ_QUEUE_ERROR;
memset(cmnd, 0, sizeof(*cmnd));
cmnd->dsm.opcode = nvme_cmd_dsm;
cmnd->dsm.nsid = cpu_to_le32(ns->ns_id);
- cmnd->dsm.nr = 0;
+ cmnd->dsm.nr = segments - 1;
cmnd->dsm.attributes = cpu_to_le32(NVME_DSMGMT_AD);
req->special_vec.bv_page = virt_to_page(range);
req->special_vec.bv_offset = offset_in_page(range);
- req->special_vec.bv_len = sizeof(*range);
+ req->special_vec.bv_len = sizeof(*range) * segments;
req->rq_flags |= RQF_SPECIAL_PAYLOAD;
return BLK_MQ_RQ_QUEUE_OK;
@@ -1233,6 +1243,8 @@ static void nvme_set_queue_limits(struct nvme_ctrl *ctrl,
if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
vwc = true;
blk_queue_write_cache(q, vwc, vwc);
+ blk_queue_max_discard_segments(q,
+ PAGE_SIZE / sizeof(struct nvme_dsm_range));
}
/*
--
2.11.0
^ permalink raw reply related
* [PATCH 3/4] block: optionally merge discontiguous discard bios into a single request
From: Christoph Hellwig @ 2017-02-07 16:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170207164658.32449-1-hch@lst.de>
Add a new merge strategy that merges discard bios into a request until the
maximum number of discard ranges (or the maximum discard size) is reached
from the plug merging code. I/O scheduler merging is not wired up yet
but might also be useful, although not for fast devices like NVMe which
are the only user for now.
Note that for now we don't support limiting the size of each discard range,
but if needed that can be added later.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-core.c | 27 +++++++++++++++++++++++++++
block/blk-merge.c | 5 ++++-
block/blk-mq.c | 3 +++
block/blk-settings.c | 20 ++++++++++++++++++++
block/blk-sysfs.c | 12 ++++++++++++
block/blk.h | 2 ++
include/linux/blkdev.h | 26 ++++++++++++++++++++++++++
include/linux/elevator.h | 1 +
8 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 75fe534861df..b9e857f4afe8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1485,6 +1485,30 @@ bool bio_attempt_front_merge(struct request_queue *q, struct request *req,
return true;
}
+bool bio_attempt_discard_merge(struct request_queue *q, struct request *req,
+ struct bio *bio)
+{
+ 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(bio) >
+ blk_rq_get_max_sectors(req, blk_rq_pos(req)))
+ goto no_merge;
+
+ req->biotail->bi_next = bio;
+ req->biotail = bio;
+ req->__data_len += bio->bi_iter.bi_size;
+ req->ioprio = ioprio_best(req->ioprio, bio_prio(bio));
+ req->nr_phys_segments = segments + 1;
+
+ blk_account_io_start(req, false);
+ return true;
+no_merge:
+ req_set_nomerge(q, req);
+ return false;
+}
+
/**
* blk_attempt_plug_merge - try to merge with %current's plugged list
* @q: request_queue new bio is being queued at
@@ -1549,6 +1573,9 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
case ELEVATOR_FRONT_MERGE:
merged = bio_attempt_front_merge(q, rq, bio);
break;
+ case ELEVATOR_DISCARD_MERGE:
+ merged = bio_attempt_discard_merge(q, rq, bio);
+ break;
default:
break;
}
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 6cbd90ad5f90..2afa262425d1 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -803,7 +803,10 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
enum elv_merge blk_try_merge(struct request *rq, struct bio *bio)
{
- if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector)
+ if (req_op(rq) == REQ_OP_DISCARD &&
+ queue_max_discard_segments(rq->q) > 1)
+ return ELEVATOR_DISCARD_MERGE;
+ else if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector)
return ELEVATOR_BACK_MERGE;
else if (blk_rq_pos(rq) - bio_sectors(bio) == bio->bi_iter.bi_sector)
return ELEVATOR_FRONT_MERGE;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9294633759d9..89cb2d224488 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -780,6 +780,9 @@ static bool blk_mq_attempt_merge(struct request_queue *q,
if (blk_mq_sched_allow_merge(q, rq, bio))
merged = bio_attempt_front_merge(q, rq, bio);
break;
+ case ELEVATOR_DISCARD_MERGE:
+ merged = bio_attempt_discard_merge(q, rq, bio);
+ break;
default:
continue;
}
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 6eb19bcbf3cb..9a68c8c19aaa 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -88,6 +88,7 @@ EXPORT_SYMBOL_GPL(blk_queue_lld_busy);
void blk_set_default_limits(struct queue_limits *lim)
{
lim->max_segments = BLK_MAX_SEGMENTS;
+ lim->max_discard_segments = 1;
lim->max_integrity_segments = 0;
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
lim->virt_boundary_mask = 0;
@@ -128,6 +129,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
/* Inherit limits from component devices */
lim->discard_zeroes_data = 1;
lim->max_segments = USHRT_MAX;
+ lim->max_discard_segments = 1;
lim->max_hw_sectors = UINT_MAX;
lim->max_segment_size = UINT_MAX;
lim->max_sectors = UINT_MAX;
@@ -337,6 +339,22 @@ void blk_queue_max_segments(struct request_queue *q, unsigned short max_segments
EXPORT_SYMBOL(blk_queue_max_segments);
/**
+ * blk_queue_max_discard_segments - set max segments for discard requests
+ * @q: the request queue for the device
+ * @max_segments: max number of segments
+ *
+ * Description:
+ * Enables a low level driver to set an upper limit on the number of
+ * segments in a discard request.
+ **/
+void blk_queue_max_discard_segments(struct request_queue *q,
+ unsigned short max_segments)
+{
+ q->limits.max_discard_segments = max_segments;
+}
+EXPORT_SYMBOL(blk_queue_max_discard_segments);
+
+/**
* blk_queue_max_segment_size - set max segment size for blk_rq_map_sg
* @q: the request queue for the device
* @max_size: max size of segment in bytes
@@ -553,6 +571,8 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
b->virt_boundary_mask);
t->max_segments = min_not_zero(t->max_segments, b->max_segments);
+ t->max_discard_segments = min_not_zero(t->max_discard_segments,
+ b->max_discard_segments);
t->max_integrity_segments = min_not_zero(t->max_integrity_segments,
b->max_integrity_segments);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 48032c4759a7..070d81bae1d5 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -121,6 +121,12 @@ static ssize_t queue_max_segments_show(struct request_queue *q, char *page)
return queue_var_show(queue_max_segments(q), (page));
}
+static ssize_t queue_max_discard_segments_show(struct request_queue *q,
+ char *page)
+{
+ return queue_var_show(queue_max_discard_segments(q), (page));
+}
+
static ssize_t queue_max_integrity_segments_show(struct request_queue *q, char *page)
{
return queue_var_show(q->limits.max_integrity_segments, (page));
@@ -545,6 +551,11 @@ static struct queue_sysfs_entry queue_max_segments_entry = {
.show = queue_max_segments_show,
};
+static struct queue_sysfs_entry queue_max_discard_segments_entry = {
+ .attr = {.name = "max_discard_segments", .mode = S_IRUGO },
+ .show = queue_max_discard_segments_show,
+};
+
static struct queue_sysfs_entry queue_max_integrity_segments_entry = {
.attr = {.name = "max_integrity_segments", .mode = S_IRUGO },
.show = queue_max_integrity_segments_show,
@@ -697,6 +708,7 @@ static struct attribute *default_attrs[] = {
&queue_max_hw_sectors_entry.attr,
&queue_max_sectors_entry.attr,
&queue_max_segments_entry.attr,
+ &queue_max_discard_segments_entry.attr,
&queue_max_integrity_segments_entry.attr,
&queue_max_segment_size_entry.attr,
&queue_iosched_entry.attr,
diff --git a/block/blk.h b/block/blk.h
index ae82f2ac4019..d1ea4bd9b9a3 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -100,6 +100,8 @@ bool bio_attempt_front_merge(struct request_queue *q, struct request *req,
struct bio *bio);
bool bio_attempt_back_merge(struct request_queue *q, struct request *req,
struct bio *bio);
+bool bio_attempt_discard_merge(struct request_queue *q, struct request *req,
+ struct bio *bio);
bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
unsigned int *request_count,
struct request **same_queue_rq);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e0bac14347e6..aecca0e7d9ca 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -331,6 +331,7 @@ struct queue_limits {
unsigned short logical_block_size;
unsigned short max_segments;
unsigned short max_integrity_segments;
+ unsigned short max_discard_segments;
unsigned char misaligned;
unsigned char discard_misaligned;
@@ -1146,6 +1147,8 @@ extern void blk_queue_bounce_limit(struct request_queue *, u64);
extern void blk_queue_max_hw_sectors(struct request_queue *, unsigned int);
extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int);
extern void blk_queue_max_segments(struct request_queue *, unsigned short);
+extern void blk_queue_max_discard_segments(struct request_queue *,
+ unsigned short);
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
extern void blk_queue_max_discard_sectors(struct request_queue *q,
unsigned int max_discard_sectors);
@@ -1189,6 +1192,15 @@ extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
extern void blk_queue_flush_queueable(struct request_queue *q, bool queueable);
extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fua);
+/*
+ * Number of physical segments as sent to the device.
+ *
+ * Normally this is the number of discontiguous data segments sent by the
+ * submitter. But for data-less command like discard we might have no
+ * actual data segments submitted, but the driver might have to add it's
+ * own special payload. In that case we still return 1 here so that this
+ * special payload will be mapped.
+ */
static inline unsigned short blk_rq_nr_phys_segments(struct request *rq)
{
if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
@@ -1196,6 +1208,15 @@ static inline unsigned short blk_rq_nr_phys_segments(struct request *rq)
return rq->nr_phys_segments;
}
+/*
+ * Number of discard segments (or ranges) the driver needs to fill in.
+ * Each discard bio merged into a request is counted as one segment.
+ */
+static inline unsigned short blk_rq_nr_discard_segments(struct request *rq)
+{
+ return max_t(unsigned short, rq->nr_phys_segments, 1);
+}
+
extern int blk_rq_map_sg(struct request_queue *, struct request *, struct scatterlist *);
extern void blk_dump_rq_flags(struct request *, char *);
extern long nr_blockdev_pages(void);
@@ -1384,6 +1405,11 @@ static inline unsigned short queue_max_segments(struct request_queue *q)
return q->limits.max_segments;
}
+static inline unsigned short queue_max_discard_segments(struct request_queue *q)
+{
+ return q->limits.max_discard_segments;
+}
+
static inline unsigned int queue_max_segment_size(struct request_queue *q)
{
return q->limits.max_segment_size;
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index b38b4e651ea6..8265b6330cc2 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -16,6 +16,7 @@ enum elv_merge {
ELEVATOR_NO_MERGE = 0,
ELEVATOR_FRONT_MERGE = 1,
ELEVATOR_BACK_MERGE = 2,
+ ELEVATOR_DISCARD_MERGE = 3,
};
typedef enum elv_merge (elevator_merge_fn) (struct request_queue *, struct request **,
--
2.11.0
^ permalink raw reply related
* [PATCH 2/4] block: enumify ELEVATOR_*_MERGE
From: Christoph Hellwig @ 2017-02-07 16:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170207164658.32449-1-hch@lst.de>
Switch these constants to an enum, and make let the compiler ensure that
all callers of blk_try_merge and elv_merge handle all potential values.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-core.c | 76 +++++++++++++++++++++++++-----------------------
block/blk-merge.c | 2 +-
block/blk-mq-sched.c | 35 +++++++++++-----------
block/blk-mq.c | 32 +++++++++-----------
block/blk.h | 2 +-
block/cfq-iosched.c | 4 +--
block/deadline-iosched.c | 12 +++-----
block/elevator.c | 10 ++++---
block/mq-deadline.c | 2 +-
include/linux/elevator.h | 28 ++++++++++--------
10 files changed, 102 insertions(+), 101 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index d161d4ab7052..75fe534861df 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1513,12 +1513,11 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
{
struct blk_plug *plug;
struct request *rq;
- bool ret = false;
struct list_head *plug_list;
plug = current->plug;
if (!plug)
- goto out;
+ return false;
*request_count = 0;
if (q->mq_ops)
@@ -1527,7 +1526,7 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
plug_list = &plug->list;
list_for_each_entry_reverse(rq, plug_list, queuelist) {
- int el_ret;
+ bool merged = false;
if (rq->q == q) {
(*request_count)++;
@@ -1543,19 +1542,22 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
if (rq->q != q || !blk_rq_merge_ok(rq, bio))
continue;
- el_ret = blk_try_merge(rq, bio);
- if (el_ret == ELEVATOR_BACK_MERGE) {
- ret = bio_attempt_back_merge(q, rq, bio);
- if (ret)
- break;
- } else if (el_ret == ELEVATOR_FRONT_MERGE) {
- ret = bio_attempt_front_merge(q, rq, bio);
- if (ret)
- break;
+ switch (blk_try_merge(rq, bio)) {
+ case ELEVATOR_BACK_MERGE:
+ merged = bio_attempt_back_merge(q, rq, bio);
+ break;
+ case ELEVATOR_FRONT_MERGE:
+ merged = bio_attempt_front_merge(q, rq, bio);
+ break;
+ default:
+ break;
}
+
+ if (merged)
+ return true;
}
-out:
- return ret;
+
+ return false;
}
unsigned int blk_plug_queued_count(struct request_queue *q)
@@ -1597,7 +1599,7 @@ void init_request_from_bio(struct request *req, struct bio *bio)
static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
{
struct blk_plug *plug;
- int el_ret, where = ELEVATOR_INSERT_SORT;
+ int where = ELEVATOR_INSERT_SORT;
struct request *req, *free;
unsigned int request_count = 0;
unsigned int wb_acct;
@@ -1635,27 +1637,29 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
spin_lock_irq(q->queue_lock);
- el_ret = elv_merge(q, &req, bio);
- if (el_ret == ELEVATOR_BACK_MERGE) {
- if (bio_attempt_back_merge(q, req, bio)) {
- elv_bio_merged(q, req, bio);
- free = attempt_back_merge(q, req);
- if (!free)
- elv_merged_request(q, req, el_ret);
- else
- __blk_put_request(q, free);
- goto out_unlock;
- }
- } else if (el_ret == ELEVATOR_FRONT_MERGE) {
- if (bio_attempt_front_merge(q, req, bio)) {
- elv_bio_merged(q, req, bio);
- free = attempt_front_merge(q, req);
- if (!free)
- elv_merged_request(q, req, el_ret);
- else
- __blk_put_request(q, free);
- goto out_unlock;
- }
+ switch (elv_merge(q, &req, bio)) {
+ case ELEVATOR_BACK_MERGE:
+ if (!bio_attempt_back_merge(q, req, bio))
+ break;
+ elv_bio_merged(q, req, bio);
+ free = attempt_back_merge(q, req);
+ if (free)
+ __blk_put_request(q, free);
+ else
+ elv_merged_request(q, req, ELEVATOR_BACK_MERGE);
+ goto out_unlock;
+ case ELEVATOR_FRONT_MERGE:
+ if (!bio_attempt_front_merge(q, req, bio))
+ break;
+ elv_bio_merged(q, req, bio);
+ free = attempt_front_merge(q, req);
+ if (free)
+ __blk_put_request(q, free);
+ else
+ elv_merged_request(q, req, ELEVATOR_FRONT_MERGE);
+ goto out_unlock;
+ default:
+ break;
}
get_rq:
diff --git a/block/blk-merge.c b/block/blk-merge.c
index c956d9e7aafd..6cbd90ad5f90 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -801,7 +801,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
return true;
}
-int blk_try_merge(struct request *rq, struct bio *bio)
+enum elv_merge blk_try_merge(struct request *rq, struct bio *bio)
{
if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector)
return ELEVATOR_BACK_MERGE;
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index ee455e7cf9d8..72d0d8361175 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -238,30 +238,29 @@ bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
struct request **merged_request)
{
struct request *rq;
- int ret;
- ret = elv_merge(q, &rq, bio);
- if (ret == ELEVATOR_BACK_MERGE) {
+ switch (elv_merge(q, &rq, bio)) {
+ case ELEVATOR_BACK_MERGE:
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
- if (bio_attempt_back_merge(q, rq, bio)) {
- *merged_request = attempt_back_merge(q, rq);
- if (!*merged_request)
- elv_merged_request(q, rq, ret);
- return true;
- }
- } else if (ret == ELEVATOR_FRONT_MERGE) {
+ if (!bio_attempt_back_merge(q, rq, bio))
+ return false;
+ *merged_request = attempt_back_merge(q, rq);
+ if (!*merged_request)
+ elv_merged_request(q, rq, ELEVATOR_BACK_MERGE);
+ return true;
+ case ELEVATOR_FRONT_MERGE:
if (!blk_mq_sched_allow_merge(q, rq, bio))
return false;
- if (bio_attempt_front_merge(q, rq, bio)) {
- *merged_request = attempt_front_merge(q, rq);
- if (!*merged_request)
- elv_merged_request(q, rq, ret);
- return true;
- }
+ if (!bio_attempt_front_merge(q, rq, bio))
+ return false;
+ *merged_request = attempt_front_merge(q, rq);
+ if (!*merged_request)
+ elv_merged_request(q, rq, ELEVATOR_FRONT_MERGE);
+ return true;
+ default:
+ return false;
}
-
- return false;
}
EXPORT_SYMBOL_GPL(blk_mq_sched_try_merge);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index be183e6115a1..9294633759d9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -763,7 +763,7 @@ static bool blk_mq_attempt_merge(struct request_queue *q,
int checked = 8;
list_for_each_entry_reverse(rq, &ctx->rq_list, queuelist) {
- int el_ret;
+ bool merged = false;
if (!checked--)
break;
@@ -771,26 +771,22 @@ static bool blk_mq_attempt_merge(struct request_queue *q,
if (!blk_rq_merge_ok(rq, bio))
continue;
- el_ret = blk_try_merge(rq, bio);
- if (el_ret == ELEVATOR_NO_MERGE)
- continue;
-
- if (!blk_mq_sched_allow_merge(q, rq, bio))
+ switch (blk_try_merge(rq, bio)) {
+ case ELEVATOR_BACK_MERGE:
+ if (blk_mq_sched_allow_merge(q, rq, bio))
+ merged = bio_attempt_back_merge(q, rq, bio);
break;
-
- if (el_ret == ELEVATOR_BACK_MERGE) {
- if (bio_attempt_back_merge(q, rq, bio)) {
- ctx->rq_merged++;
- return true;
- }
- break;
- } else if (el_ret == ELEVATOR_FRONT_MERGE) {
- if (bio_attempt_front_merge(q, rq, bio)) {
- ctx->rq_merged++;
- return true;
- }
+ case ELEVATOR_FRONT_MERGE:
+ if (blk_mq_sched_allow_merge(q, rq, bio))
+ merged = bio_attempt_front_merge(q, rq, bio);
break;
+ default:
+ continue;
}
+
+ if (merged)
+ ctx->rq_merged++;
+ return merged;
}
return false;
diff --git a/block/blk.h b/block/blk.h
index 3e08703902a9..ae82f2ac4019 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -215,7 +215,7 @@ int blk_attempt_req_merge(struct request_queue *q, struct request *rq,
void blk_recalc_rq_segments(struct request *rq);
void blk_rq_set_mixed_merge(struct request *rq);
bool blk_rq_merge_ok(struct request *rq, struct bio *bio);
-int blk_try_merge(struct request *rq, struct bio *bio);
+enum elv_merge blk_try_merge(struct request *rq, struct bio *bio);
void blk_queue_congestion_threshold(struct request_queue *q);
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index f0f29ee731e1..921262770636 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -2528,7 +2528,7 @@ static void cfq_remove_request(struct request *rq)
}
}
-static int cfq_merge(struct request_queue *q, struct request **req,
+static enum elv_merge cfq_merge(struct request_queue *q, struct request **req,
struct bio *bio)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
@@ -2544,7 +2544,7 @@ static int cfq_merge(struct request_queue *q, struct request **req,
}
static void cfq_merged_request(struct request_queue *q, struct request *req,
- int type)
+ enum elv_merge type)
{
if (type == ELEVATOR_FRONT_MERGE) {
struct cfq_queue *cfqq = RQ_CFQQ(req);
diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
index 05fc0ea25a98..c68f6bbc0dcd 100644
--- a/block/deadline-iosched.c
+++ b/block/deadline-iosched.c
@@ -120,12 +120,11 @@ static void deadline_remove_request(struct request_queue *q, struct request *rq)
deadline_del_rq_rb(dd, rq);
}
-static int
+static enum elv_merge
deadline_merge(struct request_queue *q, struct request **req, struct bio *bio)
{
struct deadline_data *dd = q->elevator->elevator_data;
struct request *__rq;
- int ret;
/*
* check for front merge
@@ -138,20 +137,17 @@ deadline_merge(struct request_queue *q, struct request **req, struct bio *bio)
BUG_ON(sector != blk_rq_pos(__rq));
if (elv_bio_merge_ok(__rq, bio)) {
- ret = ELEVATOR_FRONT_MERGE;
- goto out;
+ *req = __rq;
+ return ELEVATOR_FRONT_MERGE;
}
}
}
return ELEVATOR_NO_MERGE;
-out:
- *req = __rq;
- return ret;
}
static void deadline_merged_request(struct request_queue *q,
- struct request *req, int type)
+ struct request *req, enum elv_merge type)
{
struct deadline_data *dd = q->elevator->elevator_data;
diff --git a/block/elevator.c b/block/elevator.c
index 7e4f5880dd64..27ff1ed5a6fa 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -427,11 +427,11 @@ void elv_dispatch_add_tail(struct request_queue *q, struct request *rq)
}
EXPORT_SYMBOL(elv_dispatch_add_tail);
-int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
+enum elv_merge elv_merge(struct request_queue *q, struct request **req,
+ struct bio *bio)
{
struct elevator_queue *e = q->elevator;
struct request *__rq;
- int ret;
/*
* Levels of merges:
@@ -446,7 +446,8 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
* First try one-hit cache.
*/
if (q->last_merge && elv_bio_merge_ok(q->last_merge, bio)) {
- ret = blk_try_merge(q->last_merge, bio);
+ enum elv_merge ret = blk_try_merge(q->last_merge, bio);
+
if (ret != ELEVATOR_NO_MERGE) {
*req = q->last_merge;
return ret;
@@ -514,7 +515,8 @@ bool elv_attempt_insert_merge(struct request_queue *q, struct request *rq)
return ret;
}
-void elv_merged_request(struct request_queue *q, struct request *rq, int type)
+void elv_merged_request(struct request_queue *q, struct request *rq,
+ enum elv_merge type)
{
struct elevator_queue *e = q->elevator;
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index d68d9c273a66..236121633ca0 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -121,7 +121,7 @@ static void deadline_remove_request(struct request_queue *q, struct request *rq)
}
static void dd_request_merged(struct request_queue *q, struct request *req,
- int type)
+ enum elv_merge type)
{
struct deadline_data *dd = q->elevator->elevator_data;
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index b5825c4f06f7..b38b4e651ea6 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -9,12 +9,21 @@
struct io_cq;
struct elevator_type;
-typedef int (elevator_merge_fn) (struct request_queue *, struct request **,
+/*
+ * Return values from elevator merger
+ */
+enum elv_merge {
+ ELEVATOR_NO_MERGE = 0,
+ ELEVATOR_FRONT_MERGE = 1,
+ ELEVATOR_BACK_MERGE = 2,
+};
+
+typedef enum elv_merge (elevator_merge_fn) (struct request_queue *, struct request **,
struct bio *);
typedef void (elevator_merge_req_fn) (struct request_queue *, struct request *, struct request *);
-typedef void (elevator_merged_fn) (struct request_queue *, struct request *, int);
+typedef void (elevator_merged_fn) (struct request_queue *, struct request *, enum elv_merge);
typedef int (elevator_allow_bio_merge_fn) (struct request_queue *,
struct request *, struct bio *);
@@ -87,7 +96,7 @@ struct elevator_mq_ops {
bool (*allow_merge)(struct request_queue *, struct request *, struct bio *);
bool (*bio_merge)(struct blk_mq_hw_ctx *, struct bio *);
int (*request_merge)(struct request_queue *q, struct request **, struct bio *);
- void (*request_merged)(struct request_queue *, struct request *, int);
+ void (*request_merged)(struct request_queue *, struct request *, enum elv_merge);
void (*requests_merged)(struct request_queue *, struct request *, struct request *);
struct request *(*get_request)(struct request_queue *, unsigned int, struct blk_mq_alloc_data *);
void (*put_request)(struct request *);
@@ -166,10 +175,12 @@ extern void elv_dispatch_sort(struct request_queue *, struct request *);
extern void elv_dispatch_add_tail(struct request_queue *, struct request *);
extern void elv_add_request(struct request_queue *, struct request *, int);
extern void __elv_add_request(struct request_queue *, struct request *, int);
-extern int elv_merge(struct request_queue *, struct request **, struct bio *);
+extern enum elv_merge elv_merge(struct request_queue *, struct request **,
+ struct bio *);
extern void elv_merge_requests(struct request_queue *, struct request *,
struct request *);
-extern void elv_merged_request(struct request_queue *, struct request *, int);
+extern void elv_merged_request(struct request_queue *, struct request *,
+ enum elv_merge);
extern void elv_bio_merged(struct request_queue *q, struct request *,
struct bio *);
extern bool elv_attempt_insert_merge(struct request_queue *, struct request *);
@@ -219,13 +230,6 @@ extern void elv_rb_del(struct rb_root *, struct request *);
extern struct request *elv_rb_find(struct rb_root *, sector_t);
/*
- * Return values from elevator merger
- */
-#define ELEVATOR_NO_MERGE 0
-#define ELEVATOR_FRONT_MERGE 1
-#define ELEVATOR_BACK_MERGE 2
-
-/*
* Insertion selection
*/
#define ELEVATOR_INSERT_FRONT 1
--
2.11.0
^ permalink raw reply related
* [PATCH 1/4] block: move req_set_nomerge to blk.h
From: Christoph Hellwig @ 2017-02-07 16:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-nvme
In-Reply-To: <20170207164658.32449-1-hch@lst.de>
This makes it available outside of blk-merge.c, and inlining such a trivial
helper seems pretty useful to start with.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-merge.c | 7 -------
block/blk.h | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index a373416dbc9a..c956d9e7aafd 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -482,13 +482,6 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
}
EXPORT_SYMBOL(blk_rq_map_sg);
-static void req_set_nomerge(struct request_queue *q, struct request *req)
-{
- req->cmd_flags |= REQ_NOMERGE;
- if (req == q->last_merge)
- q->last_merge = NULL;
-}
-
static inline int ll_new_hw_segment(struct request_queue *q,
struct request *req,
struct bio *bio)
diff --git a/block/blk.h b/block/blk.h
index 4972b98d47e1..3e08703902a9 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -256,6 +256,13 @@ static inline int blk_do_io_stat(struct request *rq)
!blk_rq_is_passthrough(rq);
}
+static inline void req_set_nomerge(struct request_queue *q, struct request *req)
+{
+ req->cmd_flags |= REQ_NOMERGE;
+ if (req == q->last_merge)
+ q->last_merge = NULL;
+}
+
/*
* Internal io_context interface
*/
--
2.11.0
^ permalink raw reply related
* support for multi-range discard requests
From: Christoph Hellwig @ 2017-02-07 16:46 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-nvme
Hi all,
this series adds support for merging discontiguous discard bios into a
single request if the driver supports it. This reduces the number of
discards sent to the device by about a factor of 5-6 for typical
workloads on NVMe, and for slower devices that use I/O scheduling
the number will probably be even bigger but I've not implemented
that support yet.
^ permalink raw reply
* Re: [PATCH] nbd: freeze the queue before making changes
From: Josef Bacik @ 2017-02-07 16:36 UTC (permalink / raw)
To: linux-block, kernel-team, nbd-general, axboe
In-Reply-To: <1486396489-2911-1-git-send-email-jbacik@fb.com>
On Mon, 2017-02-06 at 10:54 -0500, Josef Bacik wrote:
> The way we make changes to the NBD device is inherently racey, as we
> could be in the middle of a request and suddenly change the number of
> connections. In practice this isn't a big deal, but with timeouts we
> have to take the config_lock in order to protect ourselves since it
> is
> important those values don't change. Fix this by freezing the queue
> before we do any of our device operations.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
Actually nevermind, this messes with my plans around reconnections
since freezing also waits for existing timeouts to fire. I'll drop
this and fix it a different way. Thanks,
Josef
^ permalink raw reply
* Re: [Lsf-pc] LSF/MM Question
From: James Bottomley @ 2017-02-07 16:12 UTC (permalink / raw)
To: Jim Mostek, lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <843987263.3966583.1486483747633@mail.yahoo.com>
On Tue, 2017-02-07 at 16:09 +0000, Jim Mostek via Lsf-pc wrote:
> wondering about the upcoming Linux Storage Filesystem & MM summint in
> March. LSF/MM Question
>
> What presentations are there so far?
LSF/MM is not really a conference, it's a summit. That means it's
going to be discussion driven rather than presentation driven. The
discussion agenda will be fluid (because issues come up as other things
are discussed) but the outline should begin to take shape a couple of
weeks beforehand.
James
^ permalink raw reply
* Re: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements
From: Jens Axboe @ 2017-02-07 16:09 UTC (permalink / raw)
To: Dexuan Cui, Bart Van Assche, hare@suse.com, hare@suse.de
Cc: hch@lst.de, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, jth@kernel.org
In-Reply-To: <MWHPR03MB2669125A47D3CB7EB530D084BF430@MWHPR03MB2669.namprd03.prod.outlook.com>
On 02/06/2017 11:29 PM, Dexuan Cui wrote:
>> From: linux-block-owner@vger.kernel.org [mailto:linux-block-
>> owner@vger.kernel.org] On Behalf Of Dexuan Cui
>> with the linux-next kernel.
>>
>> I can boot the guest with linux-next's next-20170130 without any issue,
>> but since next-20170131 I haven't succeeded in booting the guest.
>>
>> With next-20170203 (mentioned in my mail last Friday), I got the same
>> calltrace as Hannes.
>>
>> With today's linux-next (next-20170206), actually the calltrace changed to
>> the below.
>> [ 122.023036] ? remove_wait_queue+0x70/0x70
>> [ 122.051383] async_synchronize_full+0x17/0x20
>> [ 122.076925] do_init_module+0xc1/0x1f9
>> [ 122.097530] load_module+0x24bc/0x2980
>
> I don't know why it hangs here, but this is the same calltrace in my
> last-Friday mail, which contains 2 calltraces. It looks the other calltrace has
> been resolved by some changes between next-20170203 and today.
>
> Here the kernel is trying to load the Hyper-V storage driver (hv_storvsc), and
> the driver's __init and .probe have finished successfully and then the kernel
> hangs here.
>
> I believe something is broken recently, because I don't have any issue before
> Jan 31.
Can you try and bisect it?
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 17/24] fuse: Convert to separately allocated bdi
From: Jan Kara @ 2017-02-07 11:35 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, linux-block
In-Reply-To: <CAJfpeguR+swSEvyhON2oW-V+HnZeSPJLhRcSxC3avWEGGiu92A@mail.gmail.com>
On Tue 07-02-17 10:16:58, Miklos Szeredi wrote:
> On Thu, Feb 2, 2017 at 6:34 PM, Jan Kara <jack@suse.cz> wrote:
> > Allocate struct backing_dev_info separately instead of embedding it
> > inside the superblock. This unifies handling of bdi among users.
>
> Acked-by: Miklos Szeredi <mszeredi@redhat.com>
>
> A follow on patch could get rid of fc->bdi_initialized too (replace
> remaining uses with fc->sb).
Yeah, I was looking at that but was not 100% sure about it from a quick
look. I'll do that.
Honza
> >
> > CC: Miklos Szeredi <miklos@szeredi.hu>
> > CC: linux-fsdevel@vger.kernel.org
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/fuse/dev.c | 8 ++++----
> > fs/fuse/fuse_i.h | 3 ---
> > fs/fuse/inode.c | 42 +++++++++++++-----------------------------
> > 3 files changed, 17 insertions(+), 36 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 70ea57c7b6bb..1912164d57e9 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -382,8 +382,8 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
> >
> > if (fc->num_background == fc->congestion_threshold &&
> > fc->connected && fc->bdi_initialized) {
> > - clear_bdi_congested(&fc->bdi, BLK_RW_SYNC);
> > - clear_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
> > + clear_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> > + clear_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> > }
> > fc->num_background--;
> > fc->active_background--;
> > @@ -570,8 +570,8 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
> > fc->blocked = 1;
> > if (fc->num_background == fc->congestion_threshold &&
> > fc->bdi_initialized) {
> > - set_bdi_congested(&fc->bdi, BLK_RW_SYNC);
> > - set_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
> > + set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> > + set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> > }
> > list_add_tail(&req->list, &fc->bg_queue);
> > flush_bg_queue(fc);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 91307940c8ac..effab9e9607f 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -631,9 +631,6 @@ struct fuse_conn {
> > /** Negotiated minor version */
> > unsigned minor;
> >
> > - /** Backing dev info */
> > - struct backing_dev_info bdi;
> > -
> > /** Entry on the fuse_conn_list */
> > struct list_head entry;
> >
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 6fe6a88ecb4a..90bacbc87fb3 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -386,12 +386,6 @@ static void fuse_send_destroy(struct fuse_conn *fc)
> > }
> > }
> >
> > -static void fuse_bdi_destroy(struct fuse_conn *fc)
> > -{
> > - if (fc->bdi_initialized)
> > - bdi_destroy(&fc->bdi);
> > -}
> > -
> > static void fuse_put_super(struct super_block *sb)
> > {
> > struct fuse_conn *fc = get_fuse_conn_super(sb);
> > @@ -403,7 +397,6 @@ static void fuse_put_super(struct super_block *sb)
> > list_del(&fc->entry);
> > fuse_ctl_remove_conn(fc);
> > mutex_unlock(&fuse_mutex);
> > - fuse_bdi_destroy(fc);
> >
> > fuse_conn_put(fc);
> > }
> > @@ -928,7 +921,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
> > fc->no_flock = 1;
> > }
> >
> > - fc->bdi.ra_pages = min(fc->bdi.ra_pages, ra_pages);
> > + fc->sb->s_bdi->ra_pages =
> > + min(fc->sb->s_bdi->ra_pages, ra_pages);
> > fc->minor = arg->minor;
> > fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
> > fc->max_write = max_t(unsigned, 4096, fc->max_write);
> > @@ -944,7 +938,7 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
> >
> > arg->major = FUSE_KERNEL_VERSION;
> > arg->minor = FUSE_KERNEL_MINOR_VERSION;
> > - arg->max_readahead = fc->bdi.ra_pages * PAGE_SIZE;
> > + arg->max_readahead = fc->sb->s_bdi->ra_pages * PAGE_SIZE;
> > arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC |
> > FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK |
> > FUSE_SPLICE_WRITE | FUSE_SPLICE_MOVE | FUSE_SPLICE_READ |
> > @@ -976,27 +970,20 @@ static void fuse_free_conn(struct fuse_conn *fc)
> > static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
> > {
> > int err;
> > + char *suffix = "";
> >
> > - fc->bdi.name = "fuse";
> > - fc->bdi.ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
> > - /* fuse does it's own writeback accounting */
> > - fc->bdi.capabilities = BDI_CAP_NO_ACCT_WB | BDI_CAP_STRICTLIMIT;
> > -
> > - err = bdi_init(&fc->bdi);
> > + if (sb->s_bdev)
> > + suffix = "-fuseblk";
> > + err = super_setup_bdi_name(sb, "%u:%u%s", MAJOR(fc->dev),
> > + MINOR(fc->dev), suffix);
> > if (err)
> > return err;
> >
> > - fc->bdi_initialized = 1;
> > -
> > - if (sb->s_bdev) {
> > - err = bdi_register(&fc->bdi, NULL, "%u:%u-fuseblk",
> > - MAJOR(fc->dev), MINOR(fc->dev));
> > - } else {
> > - err = bdi_register_dev(&fc->bdi, fc->dev);
> > - }
> > + sb->s_bdi->ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
> > + /* fuse does it's own writeback accounting */
> > + sb->s_bdi->capabilities = BDI_CAP_NO_ACCT_WB | BDI_CAP_STRICTLIMIT;
> >
> > - if (err)
> > - return err;
> > + fc->bdi_initialized = 1;
> >
> > /*
> > * For a single fuse filesystem use max 1% of dirty +
> > @@ -1010,7 +997,7 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
> > *
> > * /sys/class/bdi/<bdi>/max_ratio
> > */
> > - bdi_set_max_ratio(&fc->bdi, 1);
> > + bdi_set_max_ratio(sb->s_bdi, 1);
> >
> > return 0;
> > }
> > @@ -1113,8 +1100,6 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> > if (err)
> > goto err_dev_free;
> >
> > - sb->s_bdi = &fc->bdi;
> > -
> > /* Handle umasking inside the fuse code */
> > if (sb->s_flags & MS_POSIXACL)
> > fc->dont_mask = 1;
> > @@ -1182,7 +1167,6 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> > err_dev_free:
> > fuse_dev_free(fud);
> > err_put_conn:
> > - fuse_bdi_destroy(fc);
> > fuse_conn_put(fc);
> > err_fput:
> > fput(file);
> > --
> > 2.10.2
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 0/4 v2] BDI lifetime fix
From: Jan Kara @ 2017-02-07 12:33 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Jan Kara, Jens Axboe, linux-block, Christoph Hellwig,
Dan Williams, Laurent Dufour, Tejun Heo
In-Reply-To: <1790906.I1Z25HtRf4@morokweng>
On Mon 06-02-17 13:26:53, Thiago Jung Bauermann wrote:
> Am Montag, 6. Februar 2017, 12:48:42 BRST schrieb Thiago Jung Bauermann:
> > 216 static inline void wb_get(struct bdi_writeback *wb)
> > 217 {
> > 218 if (wb != &wb->bdi->wb)
> > 219 percpu_ref_get(&wb->refcnt);
> > 220 }
> >
> > So it looks like wb->bdi is NULL.
>
> Sorry, looking a little deeper, it's actually wb which is NULL:
>
> ./include/linux/backing-dev.h:
> 371 return inode->i_wb;
> 0xc00000000037999c <+76>: ld r31,256(r29)
>
> ./include/linux/backing-dev-defs.h:
> 218 if (wb != &wb->bdi->wb)
> 0xc0000000003799a0 <+80>: ld r9,0(r31)
> 0xc0000000003799a4 <+84>: addi r9,r9,88
> 0xc0000000003799a8 <+88>: cmpld cr7,r31,r9
> 0xc0000000003799ac <+92>: beq cr7,0xc0000000003799e0
> <locked_inode_to_wb_and_lock_list+144>
>
> We can see above that inode->i_wb is in r31, and the machine crashed at
> 0xc0000000003799a0 so it was trying to dereference wb and crashed.
> r31 is NULL in the crash information.
Thanks for report and the analysis. After some looking into the code I see
where the problem is. Writeback code assumes inode->i_wb can never become
invalid once it is set however we still call inode_detach_wb() from
__blkdev_put(). So in a way this is a different problem but closely
related.
It seems to me that instead of calling inode_detach_wb() in __blkdev_put()
we may just switch blkdev inode to bdi->wb (it is now guaranteed to stay
around). That way bdi_unregister() can complete (destroying all writeback
structures except for bdi->wb) while block device inode can still live with
a valid i_wb structure.
CCed Tejun who is more familar with this code to verify my thoughts.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info
From: Christoph Hellwig @ 2017-02-07 9:38 UTC (permalink / raw)
To: Jason Wang
Cc: Christoph Hellwig, mst, axboe, pbonzini, virtualization,
linux-block, linux-kernel
In-Reply-To: <c9ce2425-aabb-6b05-2d00-d92ede6b18bf@redhat.com>
On Tue, Feb 07, 2017 at 03:17:02PM +0800, Jason Wang wrote:
> The check is still there.
Meh, I could swear I fixed it up. Here is an updated version:
---
>From bf5e3b7fd272aea32388570503f00d0ab592fc2a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 25 Jan 2017 13:40:21 +0100
Subject: virtio_pci: remove struct virtio_pci_vq_info
We don't really need struct virtio_pci_vq_info, as most field in there
are redundant:
- the vq backpointer is not strictly neede to start with
- the entry in the vqs list is not needed - the generic virtqueue already
has list, we only need to check if it has a callback to get the same
semantics
- we can use a simple array to look up the MSI-X vec if needed.
- That simple array now also duoble serves to replace the per_vq_vectors
flag
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 117 +++++++++++--------------------------
drivers/virtio/virtio_pci_common.h | 25 +-------
drivers/virtio/virtio_pci_legacy.c | 6 +-
drivers/virtio/virtio_pci_modern.c | 6 +-
4 files changed, 39 insertions(+), 115 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 186cbab327b8..1f9fac7dad61 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
{
struct virtio_pci_device *vp_dev = opaque;
- struct virtio_pci_vq_info *info;
irqreturn_t ret = IRQ_NONE;
- unsigned long flags;
+ struct virtqueue *vq;
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_for_each_entry(info, &vp_dev->virtqueues, node) {
- if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+ list_for_each_entry(vq, &vp_dev->vdev.vqs, list) {
+ if (vring_interrupt(irq, vq) == IRQ_HANDLED)
ret = IRQ_HANDLED;
}
- spin_unlock_irqrestore(&vp_dev->lock, flags);
return ret;
}
@@ -167,55 +164,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
return err;
}
-static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned index,
- void (*callback)(struct virtqueue *vq),
- const char *name,
- u16 msix_vec)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_vq_info *info = kmalloc(sizeof *info, GFP_KERNEL);
- struct virtqueue *vq;
- unsigned long flags;
-
- /* fill out our structure that represents an active queue */
- if (!info)
- return ERR_PTR(-ENOMEM);
-
- vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, msix_vec);
- if (IS_ERR(vq))
- goto out_info;
-
- info->vq = vq;
- if (callback) {
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_add(&info->node, &vp_dev->virtqueues);
- spin_unlock_irqrestore(&vp_dev->lock, flags);
- } else {
- INIT_LIST_HEAD(&info->node);
- }
-
- vp_dev->vqs[index] = info;
- return vq;
-
-out_info:
- kfree(info);
- return vq;
-}
-
-static void vp_del_vq(struct virtqueue *vq)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
- struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
- unsigned long flags;
-
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_del(&info->node);
- spin_unlock_irqrestore(&vp_dev->lock, flags);
-
- vp_dev->del_vq(info);
- kfree(info);
-}
-
/* the config->del_vqs() implementation */
void vp_del_vqs(struct virtio_device *vdev)
{
@@ -224,16 +172,15 @@ void vp_del_vqs(struct virtio_device *vdev)
int i;
list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
- if (vp_dev->per_vq_vectors) {
- int v = vp_dev->vqs[vq->index]->msix_vector;
+ if (vp_dev->msix_vector_map) {
+ int v = vp_dev->msix_vector_map[vq->index];
if (v != VIRTIO_MSI_NO_VECTOR)
free_irq(pci_irq_vector(vp_dev->pci_dev, v),
vq);
}
- vp_del_vq(vq);
+ vp_dev->del_vq(vq);
}
- vp_dev->per_vq_vectors = false;
if (vp_dev->intx_enabled) {
free_irq(vp_dev->pci_dev->irq, vp_dev);
@@ -261,8 +208,8 @@ void vp_del_vqs(struct virtio_device *vdev)
vp_dev->msix_names = NULL;
kfree(vp_dev->msix_affinity_masks);
vp_dev->msix_affinity_masks = NULL;
- kfree(vp_dev->vqs);
- vp_dev->vqs = NULL;
+ kfree(vp_dev->msix_vector_map);
+ vp_dev->msix_vector_map = NULL;
}
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
@@ -275,10 +222,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
u16 msix_vec;
int i, err, nvectors, allocated_vectors;
- vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
- if (!vp_dev->vqs)
- return -ENOMEM;
-
if (per_vq_vectors) {
/* Best option: one for change interrupt, one per vq. */
nvectors = 1;
@@ -294,7 +237,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (err)
goto error_find;
- vp_dev->per_vq_vectors = per_vq_vectors;
+ if (per_vq_vectors) {
+ vp_dev->msix_vector_map = kmalloc_array(nvqs,
+ sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
+ if (!vp_dev->msix_vector_map)
+ goto error_find;
+ }
+
allocated_vectors = vp_dev->msix_used_vectors;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
@@ -304,19 +253,25 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (!callbacks[i])
msix_vec = VIRTIO_MSI_NO_VECTOR;
- else if (vp_dev->per_vq_vectors)
+ else if (per_vq_vectors)
msix_vec = allocated_vectors++;
else
msix_vec = VP_MSIX_VQ_VECTOR;
- vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
+ vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
+ msix_vec);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
goto error_find;
}
- if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
+ if (!per_vq_vectors)
continue;
+ if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
+ vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
+ continue;
+ }
+
/* allocate per-vq irq if available and necessary */
snprintf(vp_dev->msix_names[msix_vec],
sizeof *vp_dev->msix_names,
@@ -326,8 +281,12 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
vring_interrupt, 0,
vp_dev->msix_names[msix_vec],
vqs[i]);
- if (err)
+ if (err) {
+ /* don't free this irq on error */
+ vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
goto error_find;
+ }
+ vp_dev->msix_vector_map[i] = msix_vec;
}
return 0;
@@ -343,23 +302,18 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i, err;
- vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
- if (!vp_dev->vqs)
- return -ENOMEM;
-
err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
dev_name(&vdev->dev), vp_dev);
if (err)
goto out_del_vqs;
vp_dev->intx_enabled = 1;
- vp_dev->per_vq_vectors = false;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
vqs[i] = NULL;
continue;
}
- vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
+ vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
VIRTIO_MSI_NO_VECTOR);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
@@ -409,16 +363,15 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
{
struct virtio_device *vdev = vq->vdev;
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
- struct cpumask *mask;
- unsigned int irq;
if (!vq->callback)
return -EINVAL;
if (vp_dev->msix_enabled) {
- mask = vp_dev->msix_affinity_masks[info->msix_vector];
- irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector);
+ int vec = vp_dev->msix_vector_map[vq->index];
+ struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
+ unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
+
if (cpu == -1)
irq_set_affinity_hint(irq, NULL);
else {
@@ -498,8 +451,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
vp_dev->vdev.dev.parent = &pci_dev->dev;
vp_dev->vdev.dev.release = virtio_pci_release_dev;
vp_dev->pci_dev = pci_dev;
- INIT_LIST_HEAD(&vp_dev->virtqueues);
- spin_lock_init(&vp_dev->lock);
/* enable the device */
rc = pci_enable_device(pci_dev);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index b2f666250ae0..2038887bdf23 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -31,17 +31,6 @@
#include <linux/highmem.h>
#include <linux/spinlock.h>
-struct virtio_pci_vq_info {
- /* the actual virtqueue */
- struct virtqueue *vq;
-
- /* the list node for the virtqueues list */
- struct list_head node;
-
- /* MSI-X vector (or none) */
- unsigned msix_vector;
-};
-
/* Our device structure */
struct virtio_pci_device {
struct virtio_device vdev;
@@ -75,13 +64,6 @@ struct virtio_pci_device {
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
- /* a list of queues so we can dispatch IRQs */
- spinlock_t lock;
- struct list_head virtqueues;
-
- /* array of all queues for house-keeping */
- struct virtio_pci_vq_info **vqs;
-
/* MSI-X support */
int msix_enabled;
int intx_enabled;
@@ -94,16 +76,15 @@ struct virtio_pci_device {
/* Vectors allocated, excluding per-vq vectors if any */
unsigned msix_used_vectors;
- /* Whether we have vector per vq */
- bool per_vq_vectors;
+ /* Map of per-VQ MSI-X vectors, may be NULL */
+ unsigned *msix_vector_map;
struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned idx,
void (*callback)(struct virtqueue *vq),
const char *name,
u16 msix_vec);
- void (*del_vq)(struct virtio_pci_vq_info *info);
+ void (*del_vq)(struct virtqueue *vq);
u16 (*config_vector)(struct virtio_pci_device *vp_dev, u16 vector);
};
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 6d9e5173d5fa..47292dad0ff9 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -112,7 +112,6 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
}
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned index,
void (*callback)(struct virtqueue *vq),
const char *name,
@@ -130,8 +129,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
if (!num || ioread32(vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN))
return ERR_PTR(-ENOENT);
- info->msix_vector = msix_vec;
-
/* create the vring */
vq = vring_create_virtqueue(index, num,
VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev,
@@ -162,9 +159,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
return ERR_PTR(err);
}
-static void del_vq(struct virtio_pci_vq_info *info)
+static void del_vq(struct virtqueue *vq)
{
- struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 4bf7ab375894..00e6fc1df407 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -293,7 +293,6 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
}
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned index,
void (*callback)(struct virtqueue *vq),
const char *name,
@@ -323,8 +322,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
/* get offset of notification word for this vq */
off = vp_ioread16(&cfg->queue_notify_off);
- info->msix_vector = msix_vec;
-
/* create the vring */
vq = vring_create_virtqueue(index, num,
SMP_CACHE_BYTES, &vp_dev->vdev,
@@ -409,9 +406,8 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
return 0;
}
-static void del_vq(struct virtio_pci_vq_info *info)
+static void del_vq(struct virtqueue *vq)
{
- struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
vp_iowrite16(vq->index, &vp_dev->common->queue_select);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 17/24] fuse: Convert to separately allocated bdi
From: Miklos Szeredi @ 2017-02-07 9:16 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig, linux-block
In-Reply-To: <20170202173422.3240-18-jack@suse.cz>
On Thu, Feb 2, 2017 at 6:34 PM, Jan Kara <jack@suse.cz> wrote:
> Allocate struct backing_dev_info separately instead of embedding it
> inside the superblock. This unifies handling of bdi among users.
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
A follow on patch could get rid of fc->bdi_initialized too (replace
remaining uses with fc->sb).
Thanks,
Miklos
>
> CC: Miklos Szeredi <miklos@szeredi.hu>
> CC: linux-fsdevel@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/fuse/dev.c | 8 ++++----
> fs/fuse/fuse_i.h | 3 ---
> fs/fuse/inode.c | 42 +++++++++++++-----------------------------
> 3 files changed, 17 insertions(+), 36 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 70ea57c7b6bb..1912164d57e9 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -382,8 +382,8 @@ static void request_end(struct fuse_conn *fc, struct fuse_req *req)
>
> if (fc->num_background == fc->congestion_threshold &&
> fc->connected && fc->bdi_initialized) {
> - clear_bdi_congested(&fc->bdi, BLK_RW_SYNC);
> - clear_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
> + clear_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> + clear_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> }
> fc->num_background--;
> fc->active_background--;
> @@ -570,8 +570,8 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
> fc->blocked = 1;
> if (fc->num_background == fc->congestion_threshold &&
> fc->bdi_initialized) {
> - set_bdi_congested(&fc->bdi, BLK_RW_SYNC);
> - set_bdi_congested(&fc->bdi, BLK_RW_ASYNC);
> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_SYNC);
> + set_bdi_congested(fc->sb->s_bdi, BLK_RW_ASYNC);
> }
> list_add_tail(&req->list, &fc->bg_queue);
> flush_bg_queue(fc);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 91307940c8ac..effab9e9607f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -631,9 +631,6 @@ struct fuse_conn {
> /** Negotiated minor version */
> unsigned minor;
>
> - /** Backing dev info */
> - struct backing_dev_info bdi;
> -
> /** Entry on the fuse_conn_list */
> struct list_head entry;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 6fe6a88ecb4a..90bacbc87fb3 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -386,12 +386,6 @@ static void fuse_send_destroy(struct fuse_conn *fc)
> }
> }
>
> -static void fuse_bdi_destroy(struct fuse_conn *fc)
> -{
> - if (fc->bdi_initialized)
> - bdi_destroy(&fc->bdi);
> -}
> -
> static void fuse_put_super(struct super_block *sb)
> {
> struct fuse_conn *fc = get_fuse_conn_super(sb);
> @@ -403,7 +397,6 @@ static void fuse_put_super(struct super_block *sb)
> list_del(&fc->entry);
> fuse_ctl_remove_conn(fc);
> mutex_unlock(&fuse_mutex);
> - fuse_bdi_destroy(fc);
>
> fuse_conn_put(fc);
> }
> @@ -928,7 +921,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
> fc->no_flock = 1;
> }
>
> - fc->bdi.ra_pages = min(fc->bdi.ra_pages, ra_pages);
> + fc->sb->s_bdi->ra_pages =
> + min(fc->sb->s_bdi->ra_pages, ra_pages);
> fc->minor = arg->minor;
> fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
> fc->max_write = max_t(unsigned, 4096, fc->max_write);
> @@ -944,7 +938,7 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
>
> arg->major = FUSE_KERNEL_VERSION;
> arg->minor = FUSE_KERNEL_MINOR_VERSION;
> - arg->max_readahead = fc->bdi.ra_pages * PAGE_SIZE;
> + arg->max_readahead = fc->sb->s_bdi->ra_pages * PAGE_SIZE;
> arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC |
> FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK |
> FUSE_SPLICE_WRITE | FUSE_SPLICE_MOVE | FUSE_SPLICE_READ |
> @@ -976,27 +970,20 @@ static void fuse_free_conn(struct fuse_conn *fc)
> static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
> {
> int err;
> + char *suffix = "";
>
> - fc->bdi.name = "fuse";
> - fc->bdi.ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
> - /* fuse does it's own writeback accounting */
> - fc->bdi.capabilities = BDI_CAP_NO_ACCT_WB | BDI_CAP_STRICTLIMIT;
> -
> - err = bdi_init(&fc->bdi);
> + if (sb->s_bdev)
> + suffix = "-fuseblk";
> + err = super_setup_bdi_name(sb, "%u:%u%s", MAJOR(fc->dev),
> + MINOR(fc->dev), suffix);
> if (err)
> return err;
>
> - fc->bdi_initialized = 1;
> -
> - if (sb->s_bdev) {
> - err = bdi_register(&fc->bdi, NULL, "%u:%u-fuseblk",
> - MAJOR(fc->dev), MINOR(fc->dev));
> - } else {
> - err = bdi_register_dev(&fc->bdi, fc->dev);
> - }
> + sb->s_bdi->ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_SIZE;
> + /* fuse does it's own writeback accounting */
> + sb->s_bdi->capabilities = BDI_CAP_NO_ACCT_WB | BDI_CAP_STRICTLIMIT;
>
> - if (err)
> - return err;
> + fc->bdi_initialized = 1;
>
> /*
> * For a single fuse filesystem use max 1% of dirty +
> @@ -1010,7 +997,7 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
> *
> * /sys/class/bdi/<bdi>/max_ratio
> */
> - bdi_set_max_ratio(&fc->bdi, 1);
> + bdi_set_max_ratio(sb->s_bdi, 1);
>
> return 0;
> }
> @@ -1113,8 +1100,6 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> if (err)
> goto err_dev_free;
>
> - sb->s_bdi = &fc->bdi;
> -
> /* Handle umasking inside the fuse code */
> if (sb->s_flags & MS_POSIXACL)
> fc->dont_mask = 1;
> @@ -1182,7 +1167,6 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> err_dev_free:
> fuse_dev_free(fud);
> err_put_conn:
> - fuse_bdi_destroy(fc);
> fuse_conn_put(fc);
> err_fput:
> fput(file);
> --
> 2.10.2
>
^ permalink raw reply
* Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Jason Wang @ 2017-02-07 7:17 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-3-hch@lst.de>
On 2017年02月06日 01:15, Christoph Hellwig wrote:
> This lets IRQ layer handle dispatching IRQs to separate handlers for the
> case where we don't have per-VQ MSI-X vectors, and allows us to greatly
> simplify the code based on the assumption that we always have interrupt
> vector 0 (legacy INTx or config interrupt for MSI-X) available, and
> any other interrupt is request/freed throught the VQ, even if the
> actual interrupt line might be shared in some cases.
>
> This allows removing a great deal of variables keeping track of the
> interrupt state in struct virtio_pci_device, as we can now simply walk the
> list of VQs and deal with per-VQ interrupt handlers there, and only treat
> vector 0 special.
>
> Additionally clean up the VQ allocation code to properly unwind on error
> instead of having a single global cleanup label, which is error prone,
> and in this case also leads to more code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/virtio/virtio_pci_common.c | 235 ++++++++++++++++---------------------
> drivers/virtio/virtio_pci_common.h | 16 +--
> 2 files changed, 106 insertions(+), 145 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index a33767318cbf..274dc1ff09c0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -33,10 +33,8 @@ void vp_synchronize_vectors(struct virtio_device *vdev)
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> int i;
>
> - if (vp_dev->intx_enabled)
> - synchronize_irq(vp_dev->pci_dev->irq);
> -
> - for (i = 0; i < vp_dev->msix_vectors; ++i)
> + synchronize_irq(pci_irq_vector(vp_dev->pci_dev, 0));
> + for (i = 1; i < vp_dev->msix_vectors; i++)
> synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
> }
>
> @@ -99,77 +97,10 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
> return vp_vring_interrupt(irq, opaque);
> }
>
> -static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
> - bool per_vq_vectors)
> -{
> - struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> - const char *name = dev_name(&vp_dev->vdev.dev);
> - unsigned i, v;
> - int err = -ENOMEM;
> -
> - vp_dev->msix_vectors = nvectors;
> -
> - vp_dev->msix_names = kmalloc(nvectors * sizeof *vp_dev->msix_names,
> - GFP_KERNEL);
> - if (!vp_dev->msix_names)
> - goto error;
> - vp_dev->msix_affinity_masks
> - = kzalloc(nvectors * sizeof *vp_dev->msix_affinity_masks,
> - GFP_KERNEL);
> - if (!vp_dev->msix_affinity_masks)
> - goto error;
> - for (i = 0; i < nvectors; ++i)
> - if (!alloc_cpumask_var(&vp_dev->msix_affinity_masks[i],
> - GFP_KERNEL))
> - goto error;
> -
> - err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
> - PCI_IRQ_MSIX);
> - if (err < 0)
> - goto error;
> - vp_dev->msix_enabled = 1;
> -
> - /* Set the vector used for configuration */
> - v = vp_dev->msix_used_vectors;
> - snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
> - "%s-config", name);
> - err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
> - vp_config_changed, 0, vp_dev->msix_names[v],
> - vp_dev);
> - if (err)
> - goto error;
> - ++vp_dev->msix_used_vectors;
> -
> - v = vp_dev->config_vector(vp_dev, v);
> - /* Verify we had enough resources to assign the vector */
> - if (v == VIRTIO_MSI_NO_VECTOR) {
> - err = -EBUSY;
> - goto error;
> - }
> -
> - if (!per_vq_vectors) {
> - /* Shared vector for all VQs */
> - v = vp_dev->msix_used_vectors;
> - snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
> - "%s-virtqueues", name);
> - err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
> - vp_vring_interrupt, 0, vp_dev->msix_names[v],
> - vp_dev);
> - if (err)
> - goto error;
> - ++vp_dev->msix_used_vectors;
> - }
> - return 0;
> -error:
> - return err;
> -}
> -
> -/* the config->del_vqs() implementation */
> -void vp_del_vqs(struct virtio_device *vdev)
> +static void vp_remove_vqs(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct virtqueue *vq, *n;
> - int i;
>
> list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
> if (vp_dev->msix_vector_map) {
> @@ -181,35 +112,33 @@ void vp_del_vqs(struct virtio_device *vdev)
> }
> vp_dev->del_vq(vq);
> }
> +}
>
> - if (vp_dev->intx_enabled) {
> - free_irq(vp_dev->pci_dev->irq, vp_dev);
> - vp_dev->intx_enabled = 0;
> - }
> +/* the config->del_vqs() implementation */
> +void vp_del_vqs(struct virtio_device *vdev)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + int i;
>
> - for (i = 0; i < vp_dev->msix_used_vectors; ++i)
> - free_irq(pci_irq_vector(vp_dev->pci_dev, i), vp_dev);
> + if (WARN_ON_ONCE(list_empty_careful(&vdev->vqs)))
> + return;
>
> - for (i = 0; i < vp_dev->msix_vectors; i++)
> - if (vp_dev->msix_affinity_masks[i])
> - free_cpumask_var(vp_dev->msix_affinity_masks[i]);
> + vp_remove_vqs(vdev);
>
> if (vp_dev->msix_enabled) {
> + for (i = 0; i < vp_dev->msix_vectors; i++)
> + free_cpumask_var(vp_dev->msix_affinity_masks[i]);
> +
> /* Disable the vector used for configuration */
> vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
>
> - pci_free_irq_vectors(vp_dev->pci_dev);
> - vp_dev->msix_enabled = 0;
> + kfree(vp_dev->msix_affinity_masks);
> + kfree(vp_dev->msix_names);
> + kfree(vp_dev->msix_vector_map);
> }
>
> - vp_dev->msix_vectors = 0;
> - vp_dev->msix_used_vectors = 0;
> - kfree(vp_dev->msix_names);
> - vp_dev->msix_names = NULL;
> - kfree(vp_dev->msix_affinity_masks);
> - vp_dev->msix_affinity_masks = NULL;
> - kfree(vp_dev->msix_vector_map);
> - vp_dev->msix_vector_map = NULL;
> + free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> + pci_free_irq_vectors(vp_dev->pci_dev);
> }
>
> static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> @@ -219,79 +148,122 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> bool per_vq_vectors)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + const char *name = dev_name(&vp_dev->vdev.dev);
> + int i, err = -ENOMEM, allocated_vectors, nvectors;
> u16 msix_vec;
> - int i, err, nvectors, allocated_vectors;
> +
> + nvectors = 1;
> + for (i = 0; i < nvqs; i++)
> + if (callbacks[i])
> + nvectors++;
>
> if (per_vq_vectors) {
> - /* Best option: one for change interrupt, one per vq. */
> - nvectors = 1;
> - for (i = 0; i < nvqs; ++i)
> - if (callbacks[i])
> - ++nvectors;
> + err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
> + PCI_IRQ_MSIX);
> } else {
> - /* Second best: one for change, shared for all vqs. */
> - nvectors = 2;
> + err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
> + PCI_IRQ_MSIX);
> }
> + if (err < 0)
> + return err;
>
> - err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors);
> + vp_dev->msix_vectors = nvectors;
> + vp_dev->msix_names = kmalloc_array(nvectors,
> + sizeof(*vp_dev->msix_names), GFP_KERNEL);
> + if (!vp_dev->msix_names)
> + goto out_free_irq_vectors;
> +
> + vp_dev->msix_affinity_masks = kcalloc(nvectors,
> + sizeof(*vp_dev->msix_affinity_masks), GFP_KERNEL);
> + if (!vp_dev->msix_affinity_masks)
> + goto out_free_msix_names;
> +
> + for (i = 0; i < nvectors; ++i) {
> + if (!alloc_cpumask_var(&vp_dev->msix_affinity_masks[i],
> + GFP_KERNEL))
> + goto out_free_msix_affinity_masks;
> + }
> +
> + /* Set the vector used for configuration */
> + snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
> + "%s-config", name);
> + err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
> + 0, vp_dev->msix_names[0], vp_dev);
> if (err)
> - goto error_find;
> + goto out_free_irq_vectors;
>
> - if (per_vq_vectors) {
> - vp_dev->msix_vector_map = kmalloc_array(nvqs,
> - sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> - if (!vp_dev->msix_vector_map)
> - goto error_find;
> + /* Verify we had enough resources to assign the vector */
> + if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
> + err = -EBUSY;
> + goto out_free_config_irq;
> }
>
> - allocated_vectors = vp_dev->msix_used_vectors;
> + vp_dev->msix_vector_map = kmalloc_array(nvqs,
> + sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> + if (!vp_dev->msix_vector_map)
> + goto out_disable_config_irq;
> +
> + allocated_vectors = 1; /* vector 0 is the config interrupt */
> for (i = 0; i < nvqs; ++i) {
> if (!names[i]) {
> vqs[i] = NULL;
> continue;
> }
>
> - if (!callbacks[i])
> - msix_vec = VIRTIO_MSI_NO_VECTOR;
> - else if (per_vq_vectors)
> - msix_vec = allocated_vectors++;
> + if (callbacks[i])
> + msix_vec = allocated_vectors;
> else
> - msix_vec = VP_MSIX_VQ_VECTOR;
> + msix_vec = VIRTIO_MSI_NO_VECTOR;
> +
> vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
> msix_vec);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> - goto error_find;
> + goto out_remove_vqs;
> }
>
> - if (!per_vq_vectors)
> - continue;
> -
> if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
> vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> continue;
> }
>
> - /* allocate per-vq irq if available and necessary */
> - snprintf(vp_dev->msix_names[msix_vec],
> - sizeof *vp_dev->msix_names,
> - "%s-%s",
> + snprintf(vp_dev->msix_names[i + 1],
> + sizeof(*vp_dev->msix_names), "%s-%s",
> dev_name(&vp_dev->vdev.dev), names[i]);
> err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> - vring_interrupt, 0,
> - vp_dev->msix_names[msix_vec],
> - vqs[i]);
> + vring_interrupt, IRQF_SHARED,
> + vp_dev->msix_names[i + 1], vqs[i]);
> if (err) {
> /* don't free this irq on error */
> vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> - goto error_find;
> + goto out_remove_vqs;
> }
> vp_dev->msix_vector_map[i] = msix_vec;
> +
> + if (per_vq_vectors)
> + allocated_vectors++;
> }
> +
> + vp_dev->msix_enabled = 1;
> return 0;
>
> -error_find:
> - vp_del_vqs(vdev);
> +out_remove_vqs:
> + vp_remove_vqs(vdev);
> + kfree(vp_dev->msix_vector_map);
> +out_disable_config_irq:
> + vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
> +out_free_config_irq:
> + free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> +out_free_msix_affinity_masks:
> + for (i = 0; i < nvectors; i++) {
> + if (vp_dev->msix_affinity_masks[i])
> + free_cpumask_var(vp_dev->msix_affinity_masks[i]);
> + }
> + kfree(vp_dev->msix_affinity_masks);
> +out_free_msix_names:
> + kfree(vp_dev->msix_names);
> +out_free_irq_vectors:
> + pci_free_irq_vectors(vp_dev->pci_dev);
> return err;
> }
>
> @@ -305,9 +277,8 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
> err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
> dev_name(&vdev->dev), vp_dev);
> if (err)
> - goto out_del_vqs;
> + return err;
>
> - vp_dev->intx_enabled = 1;
> for (i = 0; i < nvqs; ++i) {
> if (!names[i]) {
> vqs[i] = NULL;
> @@ -317,13 +288,15 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
> VIRTIO_MSI_NO_VECTOR);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> - goto out_del_vqs;
> + goto out_remove_vqs;
> }
> }
>
> return 0;
> -out_del_vqs:
> - vp_del_vqs(vdev);
> +
> +out_remove_vqs:
> + vp_remove_vqs(vdev);
> + free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> return err;
> }
>
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index 2038887bdf23..85593867e712 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -66,16 +66,12 @@ struct virtio_pci_device {
>
> /* MSI-X support */
> int msix_enabled;
> - int intx_enabled;
> cpumask_var_t *msix_affinity_masks;
> /* Name strings for interrupts. This size should be enough,
> * and I'm too lazy to allocate each name separately. */
> char (*msix_names)[256];
> - /* Number of available vectors */
> - unsigned msix_vectors;
> - /* Vectors allocated, excluding per-vq vectors if any */
> - unsigned msix_used_vectors;
> -
> + /* Total Number of MSI-X vectors (including per-VQ ones). */
> + int msix_vectors;
> /* Map of per-VQ MSI-X vectors, may be NULL */
> unsigned *msix_vector_map;
>
> @@ -89,14 +85,6 @@ struct virtio_pci_device {
> u16 (*config_vector)(struct virtio_pci_device *vp_dev, u16 vector);
> };
>
> -/* Constants for MSI-X */
> -/* Use first vector for configuration changes, second and the rest for
> - * virtqueues Thus, we need at least 2 vectors for MSI. */
> -enum {
> - VP_MSIX_CONFIG_VECTOR = 0,
> - VP_MSIX_VQ_VECTOR = 1,
> -};
> -
> /* Convert a generic virtio device to our structure */
> static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> {
^ permalink raw reply
* Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info
From: Jason Wang @ 2017-02-07 7:17 UTC (permalink / raw)
To: Christoph Hellwig, mst
Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-2-hch@lst.de>
On 2017年02月06日 01:15, Christoph Hellwig wrote:
> We don't really need struct virtio_pci_vq_info, as most field in there
> are redundant:
>
> - the vq backpointer is not strictly neede to start with
> - the entry in the vqs list is not needed - the generic virtqueue already
> has list, we only need to check if it has a callback to get the same
> semantics
> - we can use a simple array to look up the MSI-X vec if needed.
> - That simple array now also duoble serves to replace the per_vq_vectors
> flag
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
> drivers/virtio/virtio_pci_common.c | 117 +++++++++++--------------------------
> drivers/virtio/virtio_pci_common.h | 25 +-------
> drivers/virtio/virtio_pci_legacy.c | 6 +-
> drivers/virtio/virtio_pci_modern.c | 6 +-
> 4 files changed, 39 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 186cbab327b8..a33767318cbf 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
> static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
> {
> struct virtio_pci_device *vp_dev = opaque;
> - struct virtio_pci_vq_info *info;
> irqreturn_t ret = IRQ_NONE;
> - unsigned long flags;
> + struct virtqueue *vq;
>
> - spin_lock_irqsave(&vp_dev->lock, flags);
> - list_for_each_entry(info, &vp_dev->virtqueues, node) {
> - if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> + list_for_each_entry(vq, &vp_dev->vdev.vqs, list) {
> + if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)
> ret = IRQ_HANDLED;
> }
The check is still there.
Thanks
^ permalink raw reply
* RE: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements
From: Dexuan Cui @ 2017-02-07 6:29 UTC (permalink / raw)
To: Bart Van Assche, hare@suse.com, hare@suse.de, axboe@kernel.dk
Cc: hch@lst.de, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, jth@kernel.org
In-Reply-To: <MWHPR03MB26695ACBEBDB30E3E2E108FBBF430@MWHPR03MB2669.namprd03.prod.outlook.com>
PiBGcm9tOiBsaW51eC1ibG9jay1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzpsaW51eC1i
bG9jay0NCj4gb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgRGV4dWFuIEN1aQ0K
PiB3aXRoIHRoZSBsaW51eC1uZXh0IGtlcm5lbC4NCj4gDQo+IEkgY2FuIGJvb3QgdGhlIGd1ZXN0
IHdpdGggbGludXgtbmV4dCdzIG5leHQtMjAxNzAxMzAgd2l0aG91dCBhbnkgaXNzdWUsDQo+IGJ1
dCBzaW5jZSBuZXh0LTIwMTcwMTMxIEkgaGF2ZW4ndCBzdWNjZWVkZWQgaW4gYm9vdGluZyB0aGUg
Z3Vlc3QuDQo+IA0KPiBXaXRoIG5leHQtMjAxNzAyMDMgKG1lbnRpb25lZCBpbiBteSBtYWlsIGxh
c3QgRnJpZGF5KSwgSSBnb3QgdGhlIHNhbWUNCj4gY2FsbHRyYWNlIGFzIEhhbm5lcy4NCj4gDQo+
IFdpdGggdG9kYXkncyBsaW51eC1uZXh0IChuZXh0LTIwMTcwMjA2KSwgYWN0dWFsbHkgdGhlIGNh
bGx0cmFjZSBjaGFuZ2VkIHRvDQo+IHRoZSBiZWxvdy4NCj4gWyAgMTIyLjAyMzAzNl0gID8gcmVt
b3ZlX3dhaXRfcXVldWUrMHg3MC8weDcwDQo+IFsgIDEyMi4wNTEzODNdICBhc3luY19zeW5jaHJv
bml6ZV9mdWxsKzB4MTcvMHgyMA0KPiBbICAxMjIuMDc2OTI1XSAgZG9faW5pdF9tb2R1bGUrMHhj
MS8weDFmOQ0KPiBbICAxMjIuMDk3NTMwXSAgbG9hZF9tb2R1bGUrMHgyNGJjLzB4Mjk4MA0KIA0K
SSBkb24ndCBrbm93IHdoeSBpdCBoYW5ncyBoZXJlLCBidXQgdGhpcyBpcyB0aGUgc2FtZSBjYWxs
dHJhY2UgaW4gbXkNCmxhc3QtRnJpZGF5IG1haWwsIHdoaWNoIGNvbnRhaW5zIDIgY2FsbHRyYWNl
cy4gSXQgbG9va3MgdGhlIG90aGVyIGNhbGx0cmFjZSBoYXMNCmJlZW4gcmVzb2x2ZWQgYnkgc29t
ZSBjaGFuZ2VzIGJldHdlZW4gbmV4dC0yMDE3MDIwMyBhbmQgdG9kYXkuDQoNCkhlcmUgdGhlIGtl
cm5lbCBpcyB0cnlpbmcgdG8gbG9hZCB0aGUgSHlwZXItViBzdG9yYWdlIGRyaXZlciAoaHZfc3Rv
cnZzYyksIGFuZA0KdGhlIGRyaXZlcidzIF9faW5pdCBhbmQgLnByb2JlIGhhdmUgZmluaXNoZWQg
c3VjY2Vzc2Z1bGx5IGFuZCB0aGVuIHRoZSBrZXJuZWwNCmhhbmdzIGhlcmUuDQoNCkkgYmVsaWV2
ZSBzb21ldGhpbmcgaXMgYnJva2VuIHJlY2VudGx5LCBiZWNhdXNlIEkgZG9uJ3QgaGF2ZSBhbnkg
aXNzdWUgYmVmb3JlDQpKYW4gMzEuIA0KDQpUaGFua3MsDQotLSBEZXh1YW4NCg==
^ permalink raw reply
* Re: [lkp-robot] [scsi, block] 0dba1314d4: WARNING:at_fs/sysfs/dir.c:#sysfs_warn_dup
From: Dan Williams @ 2017-02-07 5:42 UTC (permalink / raw)
To: Jens Axboe
Cc: James Bottomley, Christoph Hellwig, kernel test robot,
Bart Van Assche, Martin K. Petersen, Jan Kara, Omar Sandoval,
Omar Sandoval, LKML, Jens Axboe, LKP, linux-scsi, linux-block
In-Reply-To: <01c338e2-2a30-610d-b7fa-00cb3ce2cf86@fb.com>
On Mon, Feb 6, 2017 at 8:09 PM, Jens Axboe <axboe@fb.com> wrote:
> On 02/06/2017 05:14 PM, James Bottomley wrote:
>> On Sun, 2017-02-05 at 21:13 -0800, Dan Williams wrote:
>>> On Sun, Feb 5, 2017 at 1:13 AM, Christoph Hellwig <hch@lst.de> wrote:
>>>> Dan,
>>>>
>>>> can you please quote your emails? I can't find any content
>>>> inbetween all these quotes.
>>>
>>> Sorry, I'm using gmail, but I'll switch to attaching the logs.
>>>
>>> So with help from Xiaolong I was able to reproduce this, and it does
>>> not appear to be a regression. We simply change the failure output of
>>> an existing bug. Attached is a log of the same test on v4.10-rc7
>>> (i.e. without the recent block/scsi fixes), and it shows sda being
>>> registered twice.
>>>
>>> "[ 6.647077] kobject (d5078ca4): tried to init an initialized
>>> object, something is seriously wrong."
>>>
>>> The change that "scsi, block: fix duplicate bdi name registration
>>> crashes" makes is to properly try to register sdb since the sda devt
>>> is still alive. However that's not a fix because we've managed to
>>> call blk_register_queue() twice on the same queue.
>>
>> OK, time to involve others: linux-scsi and linux-block cc'd and I've
>> inserted the log below.
>>
>> James
>>
>> ---
>>
>> [ 5.969672] scsi host0: scsi_debug: version 1.86 [20160430]
>> [ 5.969672] dev_size_mb=8, opts=0x0, submit_queues=1, statistics=0
>> [ 5.971895] scsi 0:0:0:0: Direct-Access Linux scsi_debug 0186 PQ: 0 ANSI: 7
>> [ 6.006983] sd 0:0:0:0: [sda] 16384 512-byte logical blocks: (8.39 MB/8.00 MiB)
>> [ 6.026965] sd 0:0:0:0: [sda] Write Protect is off
>> [ 6.027870] sd 0:0:0:0: [sda] Mode Sense: 73 00 10 08
>> [ 6.066962] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
>> [ 6.486962] sd 0:0:0:0: [sda] Attached SCSI disk
>> [ 6.488377] sd 0:0:0:0: [sda] Synchronizing SCSI cache
>> [ 6.489455] sd 0:0:0:0: Attached scsi generic sg0 type 0
>> [ 6.526982] sd 0:0:0:0: [sda] 16384 512-byte logical blocks: (8.39 MB/8.00 MiB)
>> [ 6.546964] sd 0:0:0:0: [sda] Write Protect is off
>> [ 6.547873] sd 0:0:0:0: [sda] Mode Sense: 73 00 10 08
>> [ 6.586963] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
>> [ 6.647077] kobject (d5078ca4): tried to init an initialized object, something is seriously wrong.
>
> So sda is probed twice, and hilarity ensues when we try to register it
> twice. I can't reproduce this, using scsi_debug and with scsi_async
> enabled.
>
> This is running linux-next? What's your .config?
>
The original failure report is here:
http://marc.info/?l=linux-kernel&m=148619222300774&w=2
...but it reproduces on current mainline with the same config. I
haven't spotted what makes scsi_debug behave like this.
^ permalink raw reply
* Re: [lkp-robot] [scsi, block] 0dba1314d4: WARNING:at_fs/sysfs/dir.c:#sysfs_warn_dup
From: Jens Axboe @ 2017-02-07 4:09 UTC (permalink / raw)
To: James Bottomley, Dan Williams, Christoph Hellwig
Cc: kernel test robot, Bart Van Assche, Martin K. Petersen, Jan Kara,
Omar Sandoval, Omar Sandoval, LKML, Jens Axboe, LKP, linux-scsi,
linux-block
In-Reply-To: <1486426467.2474.122.camel@HansenPartnership.com>
On 02/06/2017 05:14 PM, James Bottomley wrote:
> On Sun, 2017-02-05 at 21:13 -0800, Dan Williams wrote:
>> On Sun, Feb 5, 2017 at 1:13 AM, Christoph Hellwig <hch@lst.de> wrote:
>>> Dan,
>>>
>>> can you please quote your emails? I can't find any content
>>> inbetween all these quotes.
>>
>> Sorry, I'm using gmail, but I'll switch to attaching the logs.
>>
>> So with help from Xiaolong I was able to reproduce this, and it does
>> not appear to be a regression. We simply change the failure output of
>> an existing bug. Attached is a log of the same test on v4.10-rc7
>> (i.e. without the recent block/scsi fixes), and it shows sda being
>> registered twice.
>>
>> "[ 6.647077] kobject (d5078ca4): tried to init an initialized
>> object, something is seriously wrong."
>>
>> The change that "scsi, block: fix duplicate bdi name registration
>> crashes" makes is to properly try to register sdb since the sda devt
>> is still alive. However that's not a fix because we've managed to
>> call blk_register_queue() twice on the same queue.
>
> OK, time to involve others: linux-scsi and linux-block cc'd and I've
> inserted the log below.
>
> James
>
> ---
>
> [ 5.969672] scsi host0: scsi_debug: version 1.86 [20160430]
> [ 5.969672] dev_size_mb=8, opts=0x0, submit_queues=1, statistics=0
> [ 5.971895] scsi 0:0:0:0: Direct-Access Linux scsi_debug 0186 PQ: 0 ANSI: 7
> [ 6.006983] sd 0:0:0:0: [sda] 16384 512-byte logical blocks: (8.39 MB/8.00 MiB)
> [ 6.026965] sd 0:0:0:0: [sda] Write Protect is off
> [ 6.027870] sd 0:0:0:0: [sda] Mode Sense: 73 00 10 08
> [ 6.066962] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
> [ 6.486962] sd 0:0:0:0: [sda] Attached SCSI disk
> [ 6.488377] sd 0:0:0:0: [sda] Synchronizing SCSI cache
> [ 6.489455] sd 0:0:0:0: Attached scsi generic sg0 type 0
> [ 6.526982] sd 0:0:0:0: [sda] 16384 512-byte logical blocks: (8.39 MB/8.00 MiB)
> [ 6.546964] sd 0:0:0:0: [sda] Write Protect is off
> [ 6.547873] sd 0:0:0:0: [sda] Mode Sense: 73 00 10 08
> [ 6.586963] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
> [ 6.647077] kobject (d5078ca4): tried to init an initialized object, something is seriously wrong.
So sda is probed twice, and hilarity ensues when we try to register it
twice. I can't reproduce this, using scsi_debug and with scsi_async
enabled.
This is running linux-next? What's your .config?
--
Jens Axboe
^ permalink raw reply
* RE: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements
From: Dexuan Cui @ 2017-02-07 3:48 UTC (permalink / raw)
To: Bart Van Assche, hare@suse.com, hare@suse.de, axboe@kernel.dk
Cc: hch@lst.de, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, jth@kernel.org
In-Reply-To: <1486436195.2791.1.camel@sandisk.com>
[-- Attachment #1: Type: text/plain, Size: 3070 bytes --]
> From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com]
>
> On Tue, 2017-02-07 at 02:23 +0000, Dexuan Cui wrote:
> > Any news on this thread?
> >
> > The issue is still blocking Linux from booting up normally in my test. :-(
> >
> > Have we identified the faulty patch?
> > If so, at least I can try to revert it to boot up.
>
> It's interesting that you have a reproducible testcase. If you can tell me how to
> reproduce this I'll have a look at it together with Hannes.
>
> Bart.
I'm running a Ubuntu 16.04 guest on Hyper-V with the guest kernel replaced
with the linux-next kernel.
I can boot the guest with linux-next's next-20170130 without any issue,
but since next-20170131 I haven't succeeded in booting the guest.
With next-20170203 (mentioned in my mail last Friday), I got the same
calltrace as Hannes.
With today's linux-next (next-20170206), actually the calltrace changed to
the below:
(Please see the attached files for the kernel config and the full kernel log.)
(I applied Hannes's patch in this thread, but the situation remained the same.)
[ 121.824158] INFO: task systemd-udevd:91 blocked for more than 60 seconds.
[ 121.854885] Not tainted 4.10.0-rc6-next-20170206+ #1
[ 121.885004] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 121.927618] systemd-udevd D12816 91 86 0x00000000
[ 121.952912] Call Trace:
[ 121.964366] __schedule+0x2a9/0x900
[ 121.979931] schedule+0x36/0x80
[ 121.995288] async_synchronize_cookie_domain+0x91/0x130
[ 122.023036] ? remove_wait_queue+0x70/0x70
[ 122.051383] async_synchronize_full+0x17/0x20
[ 122.076925] do_init_module+0xc1/0x1f9
[ 122.097530] load_module+0x24bc/0x2980
[ 122.118418] ? ref_module+0x1c0/0x1c0
[ 122.139060] SYSC_finit_module+0xbc/0xf0
[ 122.161566] SyS_finit_module+0xe/0x10
[ 122.185397] entry_SYSCALL_64_fastpath+0x1e/0xb2
[ 122.221880] RIP: 0033:0x7f1d69105c19
[ 122.248526] RSP: 002b:00007ffe34dc3928 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 122.283349] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f1d69105c19
[ 122.315987] RDX: 0000000000000000 RSI: 00007f1d695fbe2a RDI: 000000000000000c
[ 122.354369] RBP: 00007ffe34dc2930 R08: 0000000000000000 R09: 0000000000000000
[ 122.407496] R10: 000000000000000c R11: 0000000000000246 R12: 000055f0b9b910a0
[ 122.443667] R13: 00007ffe34dc2910 R14: 0000000000000005 R15: 000000000aba9500
[ 122.475741]
[ 122.475741] Showing all locks held in the system:
[ 122.503742] 2 locks held by khungtaskd/17:
[ 122.524260] #0: (rcu_read_lock){......}, at: [<ffffffff9a10d5f1>] watchdog+0xa1/0x3d0
[ 122.569110] #1: (tasklist_lock){......}, at: [<ffffffff9a0aaf8d>] debug_show_all_locks+0x3d/0x1a0
[ 122.623903] 2 locks held by kworker/u128:1/61:
[ 122.654030] #0: ("events_unbound"){......}, at: [<ffffffff9a079035>] process_one_work+0x175/0x540
[ 122.710469] #1: ((&entry->work)){......}, at: [<ffffffff9a079035>] process_one_work+0x175/0x540
[ 122.770659]
Thanks,
-- Dexuan
[-- Attachment #2: kernel.config.zip --]
[-- Type: application/x-zip-compressed, Size: 22441 bytes --]
[-- Attachment #3: putty.log.zip --]
[-- Type: application/x-zip-compressed, Size: 10588 bytes --]
^ permalink raw reply
* Re: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements
From: Bart Van Assche @ 2017-02-07 2:56 UTC (permalink / raw)
To: decui@microsoft.com, hare@suse.com, hare@suse.de, axboe@kernel.dk
Cc: hch@lst.de, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, jth@kernel.org
In-Reply-To: <MWHPR03MB26699D4657FB67A535972208BF430@MWHPR03MB2669.namprd03.prod.outlook.com>
T24gVHVlLCAyMDE3LTAyLTA3IGF0IDAyOjIzICswMDAwLCBEZXh1YW4gQ3VpIHdyb3RlOg0KPiBB
bnkgbmV3cyBvbiB0aGlzIHRocmVhZD8NCj4gDQo+IFRoZSBpc3N1ZSBpcyBzdGlsbCBibG9ja2lu
ZyBMaW51eCBmcm9tIGJvb3RpbmcgdXAgbm9ybWFsbHkgaW4gbXkgdGVzdC4gOi0oDQo+IA0KPiBI
YXZlIHdlIGlkZW50aWZpZWQgdGhlIGZhdWx0eSBwYXRjaD8NCj4gSWYgc28sIGF0IGxlYXN0IEkg
Y2FuIHRyeSB0byByZXZlcnQgaXQgdG8gYm9vdCB1cC4NCg0KSXQncyBpbnRlcmVzdGluZyB0aGF0
IHlvdSBoYXZlIGEgcmVwcm9kdWNpYmxlIHRlc3RjYXNlLiBJZiB5b3UgY2FuIHRlbGwgbWUgaG93
IHRvDQpyZXByb2R1Y2UgdGhpcyBJJ2xsIGhhdmUgYSBsb29rIGF0IGl0IHRvZ2V0aGVyIHdpdGgg
SGFubmVzLg0KDQpCYXJ0LgpXZXN0ZXJuIERpZ2l0YWwgQ29ycG9yYXRpb24gKGFuZCBpdHMgc3Vi
c2lkaWFyaWVzKSBFLW1haWwgQ29uZmlkZW50aWFsaXR5IE5vdGljZSAmIERpc2NsYWltZXI6CgpU
aGlzIGUtbWFpbCBhbmQgYW55IGZpbGVzIHRyYW5zbWl0dGVkIHdpdGggaXQgbWF5IGNvbnRhaW4g
Y29uZmlkZW50aWFsIG9yIGxlZ2FsbHkgcHJpdmlsZWdlZCBpbmZvcm1hdGlvbiBvZiBXREMgYW5k
L29yIGl0cyBhZmZpbGlhdGVzLCBhbmQgYXJlIGludGVuZGVkIHNvbGVseSBmb3IgdGhlIHVzZSBv
ZiB0aGUgaW5kaXZpZHVhbCBvciBlbnRpdHkgdG8gd2hpY2ggdGhleSBhcmUgYWRkcmVzc2VkLiBJ
ZiB5b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQgcmVjaXBpZW50LCBhbnkgZGlzY2xvc3VyZSwgY29w
eWluZywgZGlzdHJpYnV0aW9uIG9yIGFueSBhY3Rpb24gdGFrZW4gb3Igb21pdHRlZCB0byBiZSB0
YWtlbiBpbiByZWxpYW5jZSBvbiBpdCwgaXMgcHJvaGliaXRlZC4gSWYgeW91IGhhdmUgcmVjZWl2
ZWQgdGhpcyBlLW1haWwgaW4gZXJyb3IsIHBsZWFzZSBub3RpZnkgdGhlIHNlbmRlciBpbW1lZGlh
dGVseSBhbmQgZGVsZXRlIHRoZSBlLW1haWwgaW4gaXRzIGVudGlyZXR5IGZyb20geW91ciBzeXN0
ZW0uCg==
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox