* [PATCH v4 01/10] xen/blkif: document blkif multi-queue/ring extension
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-02 4:21 ` [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info Bob Liu
` (9 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Document the multi-queue/ring feature in terms of XenStore keys to be written by
the backend and by the frontend.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
--
v2:
Add descriptions together with multi-page ring buffer.
---
include/xen/interface/io/blkif.h | 48 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index c33e1c4..8b8cfad 100644
--- a/include/xen/interface/io/blkif.h
+++ b/include/xen/interface/io/blkif.h
@@ -28,6 +28,54 @@ typedef uint16_t blkif_vdev_t;
typedef uint64_t blkif_sector_t;
/*
+ * Multiple hardware queues/rings:
+ * If supported, the backend will write the key "multi-queue-max-queues" to
+ * the directory for that vbd, and set its value to the maximum supported
+ * number of queues.
+ * Frontends that are aware of this feature and wish to use it can write the
+ * key "multi-queue-num-queues" with the number they wish to use, which must be
+ * greater than zero, and no more than the value reported by the backend in
+ * "multi-queue-max-queues".
+ *
+ * For frontends requesting just one queue, the usual event-channel and
+ * ring-ref keys are written as before, simplifying the backend processing
+ * to avoid distinguishing between a frontend that doesn't understand the
+ * multi-queue feature, and one that does, but requested only one queue.
+ *
+ * Frontends requesting two or more queues must not write the toplevel
+ * event-channel and ring-ref keys, instead writing those keys under sub-keys
+ * having the name "queue-N" where N is the integer ID of the queue/ring for
+ * which those keys belong. Queues are indexed from zero.
+ * For example, a frontend with two queues must write the following set of
+ * queue-related keys:
+ *
+ * /local/domain/1/device/vbd/0/multi-queue-num-queues = "2"
+ * /local/domain/1/device/vbd/0/queue-0 = ""
+ * /local/domain/1/device/vbd/0/queue-0/ring-ref = "<ring-ref#0>"
+ * /local/domain/1/device/vbd/0/queue-0/event-channel = "<evtchn#0>"
+ * /local/domain/1/device/vbd/0/queue-1 = ""
+ * /local/domain/1/device/vbd/0/queue-1/ring-ref = "<ring-ref#1>"
+ * /local/domain/1/device/vbd/0/queue-1/event-channel = "<evtchn#1>"
+ *
+ * It is also possible to use multiple queues/rings together with
+ * feature multi-page ring buffer.
+ * For example, a frontend requests two queues/rings and the size of each ring
+ * buffer is two pages must write the following set of related keys:
+ *
+ * /local/domain/1/device/vbd/0/multi-queue-num-queues = "2"
+ * /local/domain/1/device/vbd/0/ring-page-order = "1"
+ * /local/domain/1/device/vbd/0/queue-0 = ""
+ * /local/domain/1/device/vbd/0/queue-0/ring-ref0 = "<ring-ref#0>"
+ * /local/domain/1/device/vbd/0/queue-0/ring-ref1 = "<ring-ref#1>"
+ * /local/domain/1/device/vbd/0/queue-0/event-channel = "<evtchn#0>"
+ * /local/domain/1/device/vbd/0/queue-1 = ""
+ * /local/domain/1/device/vbd/0/queue-1/ring-ref0 = "<ring-ref#2>"
+ * /local/domain/1/device/vbd/0/queue-1/ring-ref1 = "<ring-ref#3>"
+ * /local/domain/1/device/vbd/0/queue-1/event-channel = "<evtchn#1>"
+ *
+ */
+
+/*
* REQUEST CODES.
*/
#define BLKIF_OP_READ 0
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
2015-11-02 4:21 ` [PATCH v4 01/10] xen/blkif: document blkif multi-queue/ring extension Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-02 4:49 ` kbuild test robot
2015-11-02 4:21 ` [PATCH v4 03/10] xen/blkfront: pseudo support for multi hardware queues/rings Bob Liu
` (8 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Split per ring information to an new structure "blkfront_ring_info".
A ring is the representation of a hardware queue, every vbd device can associate
with one or more rings depending on how many hardware queues/rings to be used.
This patch is a preparation for supporting real multi hardware queues/rings.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkfront.c | 321 ++++++++++++++++++++++++-------------------
1 file changed, 178 insertions(+), 143 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index a69c02d..2a557e4 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -115,6 +115,23 @@ MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the
#define RINGREF_NAME_LEN (20)
/*
+ * Per-ring info.
+ * Every blkfront device can associate with one or more blkfront_ring_info,
+ * depending on how many hardware queues/rings to be used.
+ */
+struct blkfront_ring_info {
+ struct blkif_front_ring ring;
+ unsigned int ring_ref[XENBUS_MAX_RING_PAGES];
+ unsigned int evtchn, irq;
+ struct work_struct work;
+ struct gnttab_free_callback callback;
+ struct blk_shadow shadow[BLK_MAX_RING_SIZE];
+ struct list_head indirect_pages;
+ unsigned long shadow_free;
+ struct blkfront_info *dev_info;
+};
+
+/*
* We have one of these per vbd, whether ide, scsi or 'other'. They
* hang in private_data off the gendisk structure. We may end up
* putting all kinds of interesting stuff here :-)
@@ -128,18 +145,10 @@ struct blkfront_info
int vdevice;
blkif_vdev_t handle;
enum blkif_state connected;
- int ring_ref[XENBUS_MAX_RING_PAGES];
unsigned int nr_ring_pages;
- struct blkif_front_ring ring;
- unsigned int evtchn, irq;
struct request_queue *rq;
- struct work_struct work;
- struct gnttab_free_callback callback;
- struct blk_shadow shadow[BLK_MAX_RING_SIZE];
struct list_head grants;
- struct list_head indirect_pages;
unsigned int persistent_gnts_c;
- unsigned long shadow_free;
unsigned int feature_flush;
unsigned int feature_discard:1;
unsigned int feature_secdiscard:1;
@@ -149,6 +158,7 @@ struct blkfront_info
unsigned int max_indirect_segments;
int is_ready;
struct blk_mq_tag_set tag_set;
+ struct blkfront_ring_info rinfo;
};
static unsigned int nr_minors;
@@ -179,33 +189,35 @@ static DEFINE_SPINLOCK(minor_lock);
#define INDIRECT_GREFS(_segs) \
((_segs + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
-static int blkfront_setup_indirect(struct blkfront_info *info);
+static int blkfront_setup_indirect(struct blkfront_ring_info *rinfo);
static int blkfront_gather_backend_features(struct blkfront_info *info);
-static int get_id_from_freelist(struct blkfront_info *info)
+static int get_id_from_freelist(struct blkfront_ring_info *rinfo)
{
- unsigned long free = info->shadow_free;
- BUG_ON(free >= BLK_RING_SIZE(info));
- info->shadow_free = info->shadow[free].req.u.rw.id;
- info->shadow[free].req.u.rw.id = 0x0fffffee; /* debug */
+ unsigned long free = rinfo->shadow_free;
+
+ BUG_ON(free >= BLK_RING_SIZE(rinfo->dev_info));
+ rinfo->shadow_free = rinfo->shadow[free].req.u.rw.id;
+ rinfo->shadow[free].req.u.rw.id = 0x0fffffee; /* debug */
return free;
}
-static int add_id_to_freelist(struct blkfront_info *info,
+static int add_id_to_freelist(struct blkfront_ring_info *rinfo,
unsigned long id)
{
- if (info->shadow[id].req.u.rw.id != id)
+ if (rinfo->shadow[id].req.u.rw.id != id)
return -EINVAL;
- if (info->shadow[id].request == NULL)
+ if (rinfo->shadow[id].request == NULL)
return -EINVAL;
- info->shadow[id].req.u.rw.id = info->shadow_free;
- info->shadow[id].request = NULL;
- info->shadow_free = id;
+ rinfo->shadow[id].req.u.rw.id = rinfo->shadow_free;
+ rinfo->shadow[id].request = NULL;
+ rinfo->shadow_free = id;
return 0;
}
-static int fill_grant_buffer(struct blkfront_info *info, int num)
+static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
{
+ struct blkfront_info *info = rinfo->dev_info;
struct page *granted_page;
struct grant *gnt_list_entry, *n;
int i = 0;
@@ -341,8 +353,8 @@ static void xlbd_release_minors(unsigned int minor, unsigned int nr)
static void blkif_restart_queue_callback(void *arg)
{
- struct blkfront_info *info = (struct blkfront_info *)arg;
- schedule_work(&info->work);
+ struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)arg;
+ schedule_work(&rinfo->work);
}
static int blkif_getgeo(struct block_device *bd, struct hd_geometry *hg)
@@ -400,7 +412,7 @@ static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
*
* @req: a request struct
*/
-static int blkif_queue_request(struct request *req)
+static int blkif_queue_request(struct request *req, struct blkfront_ring_info *rinfo)
{
struct blkfront_info *info = req->rq_disk->private_data;
struct blkif_request *ring_req;
@@ -438,9 +450,9 @@ static int blkif_queue_request(struct request *req)
max_grefs - info->persistent_gnts_c,
&gref_head) < 0) {
gnttab_request_free_callback(
- &info->callback,
+ &rinfo->callback,
blkif_restart_queue_callback,
- info,
+ rinfo,
max_grefs);
return 1;
}
@@ -448,9 +460,9 @@ static int blkif_queue_request(struct request *req)
new_persistent_gnts = 0;
/* Fill out a communications ring structure. */
- ring_req = RING_GET_REQUEST(&info->ring, info->ring.req_prod_pvt);
- id = get_id_from_freelist(info);
- info->shadow[id].request = req;
+ ring_req = RING_GET_REQUEST(&rinfo->ring, rinfo->ring.req_prod_pvt);
+ id = get_id_from_freelist(rinfo);
+ rinfo->shadow[id].request = req;
if (unlikely(req->cmd_flags & (REQ_DISCARD | REQ_SECURE))) {
ring_req->operation = BLKIF_OP_DISCARD;
@@ -466,7 +478,7 @@ static int blkif_queue_request(struct request *req)
req->nr_phys_segments > BLKIF_MAX_SEGMENTS_PER_REQUEST);
BUG_ON(info->max_indirect_segments &&
req->nr_phys_segments > info->max_indirect_segments);
- nseg = blk_rq_map_sg(req->q, req, info->shadow[id].sg);
+ nseg = blk_rq_map_sg(req->q, req, rinfo->shadow[id].sg);
ring_req->u.rw.id = id;
if (nseg > BLKIF_MAX_SEGMENTS_PER_REQUEST) {
/*
@@ -509,7 +521,7 @@ static int blkif_queue_request(struct request *req)
}
ring_req->u.rw.nr_segments = nseg;
}
- for_each_sg(info->shadow[id].sg, sg, nseg, i) {
+ for_each_sg(rinfo->shadow[id].sg, sg, nseg, i) {
fsect = sg->offset >> 9;
lsect = fsect + (sg->length >> 9) - 1;
@@ -525,14 +537,14 @@ static int blkif_queue_request(struct request *req)
struct page *indirect_page;
/* Fetch a pre-allocated page to use for indirect grefs */
- BUG_ON(list_empty(&info->indirect_pages));
- indirect_page = list_first_entry(&info->indirect_pages,
+ BUG_ON(list_empty(&rinfo->indirect_pages));
+ indirect_page = list_first_entry(&rinfo->indirect_pages,
struct page, lru);
list_del(&indirect_page->lru);
pfn = page_to_pfn(indirect_page);
}
gnt_list_entry = get_grant(&gref_head, pfn, info);
- info->shadow[id].indirect_grants[n] = gnt_list_entry;
+ rinfo->shadow[id].indirect_grants[n] = gnt_list_entry;
segments = kmap_atomic(pfn_to_page(gnt_list_entry->pfn));
ring_req->u.indirect.indirect_grefs[n] = gnt_list_entry->gref;
}
@@ -540,7 +552,7 @@ static int blkif_queue_request(struct request *req)
gnt_list_entry = get_grant(&gref_head, page_to_pfn(sg_page(sg)), info);
ref = gnt_list_entry->gref;
- info->shadow[id].grants_used[i] = gnt_list_entry;
+ rinfo->shadow[id].grants_used[i] = gnt_list_entry;
if (rq_data_dir(req) && info->feature_persistent) {
char *bvec_data;
@@ -586,10 +598,10 @@ static int blkif_queue_request(struct request *req)
kunmap_atomic(segments);
}
- info->ring.req_prod_pvt++;
+ rinfo->ring.req_prod_pvt++;
/* Keep a private copy so we can reissue requests when recovering. */
- info->shadow[id].req = *ring_req;
+ rinfo->shadow[id].req = *ring_req;
if (new_persistent_gnts)
gnttab_free_grant_references(gref_head);
@@ -598,14 +610,14 @@ static int blkif_queue_request(struct request *req)
}
-static inline void flush_requests(struct blkfront_info *info)
+static inline void flush_requests(struct blkfront_ring_info *rinfo)
{
int notify;
- RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&info->ring, notify);
+ RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&rinfo->ring, notify);
if (notify)
- notify_remote_via_irq(info->irq);
+ notify_remote_via_irq(rinfo->irq);
}
static inline bool blkif_request_flush_invalid(struct request *req,
@@ -621,20 +633,20 @@ static inline bool blkif_request_flush_invalid(struct request *req,
static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
const struct blk_mq_queue_data *qd)
{
- struct blkfront_info *info = qd->rq->rq_disk->private_data;
+ struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)hctx->driver_data;
blk_mq_start_request(qd->rq);
spin_lock_irq(&info->io_lock);
- if (RING_FULL(&info->ring))
+ if (RING_FULL(&rinfo->ring))
goto out_busy;
- if (blkif_request_flush_invalid(qd->rq, info))
+ if (blkif_request_flush_invalid(qd->rq, rinfo->dev_info))
goto out_err;
- if (blkif_queue_request(qd->rq))
+ if (blkif_queue_request(qd->rq, rinfo))
goto out_busy;
- flush_requests(info);
+ flush_requests(rinfo);
spin_unlock_irq(&info->io_lock);
return BLK_MQ_RQ_QUEUE_OK;
@@ -648,9 +660,19 @@ out_busy:
return BLK_MQ_RQ_QUEUE_BUSY;
}
+static int blk_mq_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
+ unsigned int index)
+{
+ struct blkfront_info *info = (struct blkfront_info *)data;
+
+ hctx->driver_data = &info->rinfo;
+ return 0;
+}
+
static struct blk_mq_ops blkfront_mq_ops = {
.queue_rq = blkif_queue_rq,
.map_queue = blk_mq_map_queue,
+ .init_hctx = blk_mq_init_hctx,
};
static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
@@ -903,6 +925,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
static void xlvbd_release_gendisk(struct blkfront_info *info)
{
unsigned int minor, nr_minors;
+ struct blkfront_ring_info *rinfo = &info->rinfo;
if (info->rq == NULL)
return;
@@ -911,10 +934,10 @@ static void xlvbd_release_gendisk(struct blkfront_info *info)
blk_mq_stop_hw_queues(info->rq);
/* No more gnttab callback work. */
- gnttab_cancel_free_callback(&info->callback);
+ gnttab_cancel_free_callback(&rinfo->callback);
/* Flush gnttab callback work. Must be done with no locks held. */
- flush_work(&info->work);
+ flush_work(&rinfo->work);
del_gendisk(info->gd);
@@ -931,20 +954,20 @@ static void xlvbd_release_gendisk(struct blkfront_info *info)
}
/* Must be called with io_lock holded */
-static void kick_pending_request_queues(struct blkfront_info *info)
+static void kick_pending_request_queues(struct blkfront_ring_info *rinfo)
{
- if (!RING_FULL(&info->ring))
- blk_mq_start_stopped_hw_queues(info->rq, true);
+ if (!RING_FULL(&rinfo->ring))
+ blk_mq_start_stopped_hw_queues(rinfo->dev_info->rq, true);
}
static void blkif_restart_queue(struct work_struct *work)
{
- struct blkfront_info *info = container_of(work, struct blkfront_info, work);
+ struct blkfront_ring_info *rinfo = container_of(work, struct blkfront_ring_info, work);
- spin_lock_irq(&info->io_lock);
- if (info->connected == BLKIF_STATE_CONNECTED)
- kick_pending_request_queues(info);
- spin_unlock_irq(&info->io_lock);
+ spin_lock_irq(&rinfo->dev_info->io_lock);
+ if (rinfo->dev_info->connected == BLKIF_STATE_CONNECTED)
+ kick_pending_request_queues(rinfo);
+ spin_unlock_irq(&rinfo->dev_info->io_lock);
}
static void blkif_free(struct blkfront_info *info, int suspend)
@@ -952,6 +975,7 @@ static void blkif_free(struct blkfront_info *info, int suspend)
struct grant *persistent_gnt;
struct grant *n;
int i, j, segs;
+ struct blkfront_ring_info *rinfo = &info->rinfo;
/* Prevent new requests being issued until we fix things up. */
spin_lock_irq(&info->io_lock);
@@ -964,7 +988,7 @@ static void blkif_free(struct blkfront_info *info, int suspend)
/* Remove all persistent grants */
if (!list_empty(&info->grants)) {
list_for_each_entry_safe(persistent_gnt, n,
- &info->grants, node) {
+ &info->grants, node) {
list_del(&persistent_gnt->node);
if (persistent_gnt->gref != GRANT_INVALID_REF) {
gnttab_end_foreign_access(persistent_gnt->gref,
@@ -982,11 +1006,11 @@ static void blkif_free(struct blkfront_info *info, int suspend)
* Remove indirect pages, this only happens when using indirect
* descriptors but not persistent grants
*/
- if (!list_empty(&info->indirect_pages)) {
+ if (!list_empty(&rinfo->indirect_pages)) {
struct page *indirect_page, *n;
BUG_ON(info->feature_persistent);
- list_for_each_entry_safe(indirect_page, n, &info->indirect_pages, lru) {
+ list_for_each_entry_safe(indirect_page, n, &rinfo->indirect_pages, lru) {
list_del(&indirect_page->lru);
__free_page(indirect_page);
}
@@ -997,21 +1021,21 @@ static void blkif_free(struct blkfront_info *info, int suspend)
* Clear persistent grants present in requests already
* on the shared ring
*/
- if (!info->shadow[i].request)
+ if (!rinfo->shadow[i].request)
goto free_shadow;
- segs = info->shadow[i].req.operation == BLKIF_OP_INDIRECT ?
- info->shadow[i].req.u.indirect.nr_segments :
- info->shadow[i].req.u.rw.nr_segments;
+ segs = rinfo->shadow[i].req.operation == BLKIF_OP_INDIRECT ?
+ rinfo->shadow[i].req.u.indirect.nr_segments :
+ rinfo->shadow[i].req.u.rw.nr_segments;
for (j = 0; j < segs; j++) {
- persistent_gnt = info->shadow[i].grants_used[j];
+ persistent_gnt = rinfo->shadow[i].grants_used[j];
gnttab_end_foreign_access(persistent_gnt->gref, 0, 0UL);
if (info->feature_persistent)
__free_page(pfn_to_page(persistent_gnt->pfn));
kfree(persistent_gnt);
}
- if (info->shadow[i].req.operation != BLKIF_OP_INDIRECT)
+ if (rinfo->shadow[i].req.operation != BLKIF_OP_INDIRECT)
/*
* If this is not an indirect operation don't try to
* free indirect segments
@@ -1019,45 +1043,45 @@ static void blkif_free(struct blkfront_info *info, int suspend)
goto free_shadow;
for (j = 0; j < INDIRECT_GREFS(segs); j++) {
- persistent_gnt = info->shadow[i].indirect_grants[j];
+ persistent_gnt = rinfo->shadow[i].indirect_grants[j];
gnttab_end_foreign_access(persistent_gnt->gref, 0, 0UL);
__free_page(pfn_to_page(persistent_gnt->pfn));
kfree(persistent_gnt);
}
free_shadow:
- kfree(info->shadow[i].grants_used);
- info->shadow[i].grants_used = NULL;
- kfree(info->shadow[i].indirect_grants);
- info->shadow[i].indirect_grants = NULL;
- kfree(info->shadow[i].sg);
- info->shadow[i].sg = NULL;
+ kfree(rinfo->shadow[i].grants_used);
+ rinfo->shadow[i].grants_used = NULL;
+ kfree(rinfo->shadow[i].indirect_grants);
+ rinfo->shadow[i].indirect_grants = NULL;
+ kfree(rinfo->shadow[i].sg);
+ rinfo->shadow[i].sg = NULL;
}
/* No more gnttab callback work. */
- gnttab_cancel_free_callback(&info->callback);
+ gnttab_cancel_free_callback(&rinfo->callback);
spin_unlock_irq(&info->io_lock);
/* Flush gnttab callback work. Must be done with no locks held. */
- flush_work(&info->work);
+ flush_work(&rinfo->work);
/* Free resources associated with old device channel. */
for (i = 0; i < info->nr_ring_pages; i++) {
- if (info->ring_ref[i] != GRANT_INVALID_REF) {
- gnttab_end_foreign_access(info->ring_ref[i], 0, 0);
- info->ring_ref[i] = GRANT_INVALID_REF;
+ if (rinfo->ring_ref[i] != GRANT_INVALID_REF) {
+ gnttab_end_foreign_access(rinfo->ring_ref[i], 0, 0);
+ rinfo->ring_ref[i] = GRANT_INVALID_REF;
}
}
- free_pages((unsigned long)info->ring.sring, get_order(info->nr_ring_pages * PAGE_SIZE));
- info->ring.sring = NULL;
+ free_pages((unsigned long)rinfo->ring.sring, get_order(info->nr_ring_pages * PAGE_SIZE));
+ rinfo->ring.sring = NULL;
- if (info->irq)
- unbind_from_irqhandler(info->irq, info);
- info->evtchn = info->irq = 0;
+ if (rinfo->irq)
+ unbind_from_irqhandler(rinfo->irq, rinfo);
+ rinfo->evtchn = rinfo->irq = 0;
}
-static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
+static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *rinfo,
struct blkif_response *bret)
{
int i = 0;
@@ -1065,6 +1089,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
char *bvec_data;
void *shared_data;
int nseg;
+ struct blkfront_info *info = rinfo->dev_info;
nseg = s->req.operation == BLKIF_OP_INDIRECT ?
s->req.u.indirect.nr_segments : s->req.u.rw.nr_segments;
@@ -1126,7 +1151,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
*/
if (!info->feature_persistent) {
indirect_page = pfn_to_page(s->indirect_grants[i]->pfn);
- list_add(&indirect_page->lru, &info->indirect_pages);
+ list_add(&indirect_page->lru, &rinfo->indirect_pages);
}
s->indirect_grants[i]->gref = GRANT_INVALID_REF;
list_add_tail(&s->indirect_grants[i]->node, &info->grants);
@@ -1141,7 +1166,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
struct blkif_response *bret;
RING_IDX i, rp;
unsigned long flags;
- struct blkfront_info *info = (struct blkfront_info *)dev_id;
+ struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)dev_id;
+ struct blkfront_info *info = rinfo->dev_info;
int error;
spin_lock_irqsave(&info->io_lock, flags);
@@ -1152,13 +1178,13 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
}
again:
- rp = info->ring.sring->rsp_prod;
+ rp = rinfo->ring.sring->rsp_prod;
rmb(); /* Ensure we see queued responses up to 'rp'. */
- for (i = info->ring.rsp_cons; i != rp; i++) {
+ for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
- bret = RING_GET_RESPONSE(&info->ring, i);
+ bret = RING_GET_RESPONSE(&rinfo->ring, i);
id = bret->id;
/*
* The backend has messed up and given us an id that we would
@@ -1172,12 +1198,12 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
* the id is busted. */
continue;
}
- req = info->shadow[id].request;
+ req = rinfo->shadow[id].request;
if (bret->operation != BLKIF_OP_DISCARD)
- blkif_completion(&info->shadow[id], info, bret);
+ blkif_completion(&rinfo->shadow[id], rinfo, bret);
- if (add_id_to_freelist(info, id)) {
+ if (add_id_to_freelist(rinfo, id)) {
WARN(1, "%s: response to %s (id %ld) couldn't be recycled!\n",
info->gd->disk_name, op_name(bret->operation), id);
continue;
@@ -1206,7 +1232,7 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
error = -EOPNOTSUPP;
}
if (unlikely(bret->status == BLKIF_RSP_ERROR &&
- info->shadow[id].req.u.rw.nr_segments == 0)) {
+ rinfo->shadow[id].req.u.rw.nr_segments == 0)) {
printk(KERN_WARNING "blkfront: %s: empty %s op failed\n",
info->gd->disk_name, op_name(bret->operation));
error = -EOPNOTSUPP;
@@ -1231,17 +1257,17 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
}
}
- info->ring.rsp_cons = i;
+ rinfo->ring.rsp_cons = i;
- if (i != info->ring.req_prod_pvt) {
+ if (i != rinfo->ring.req_prod_pvt) {
int more_to_do;
- RING_FINAL_CHECK_FOR_RESPONSES(&info->ring, more_to_do);
+ RING_FINAL_CHECK_FOR_RESPONSES(&rinfo->ring, more_to_do);
if (more_to_do)
goto again;
} else
- info->ring.sring->rsp_event = i + 1;
+ rinfo->ring.sring->rsp_event = i + 1;
- kick_pending_request_queues(info);
+ kick_pending_request_queues(rinfo);
spin_unlock_irqrestore(&info->io_lock, flags);
@@ -1250,15 +1276,16 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
static int setup_blkring(struct xenbus_device *dev,
- struct blkfront_info *info)
+ struct blkfront_ring_info *rinfo)
{
struct blkif_sring *sring;
int err, i;
+ struct blkfront_info *info = rinfo->dev_info;
unsigned long ring_size = info->nr_ring_pages * PAGE_SIZE;
grant_ref_t gref[XENBUS_MAX_RING_PAGES];
for (i = 0; i < info->nr_ring_pages; i++)
- info->ring_ref[i] = GRANT_INVALID_REF;
+ rinfo->ring_ref[i] = GRANT_INVALID_REF;
sring = (struct blkif_sring *)__get_free_pages(GFP_NOIO | __GFP_HIGH,
get_order(ring_size));
@@ -1267,29 +1294,29 @@ static int setup_blkring(struct xenbus_device *dev,
return -ENOMEM;
}
SHARED_RING_INIT(sring);
- FRONT_RING_INIT(&info->ring, sring, ring_size);
+ FRONT_RING_INIT(&rinfo->ring, sring, ring_size);
- err = xenbus_grant_ring(dev, info->ring.sring, info->nr_ring_pages, gref);
+ err = xenbus_grant_ring(dev, rinfo->ring.sring, info->nr_ring_pages, gref);
if (err < 0) {
free_pages((unsigned long)sring, get_order(ring_size));
- info->ring.sring = NULL;
+ rinfo->ring.sring = NULL;
goto fail;
}
for (i = 0; i < info->nr_ring_pages; i++)
- info->ring_ref[i] = gref[i];
+ rinfo->ring_ref[i] = gref[i];
- err = xenbus_alloc_evtchn(dev, &info->evtchn);
+ err = xenbus_alloc_evtchn(dev, &rinfo->evtchn);
if (err)
goto fail;
- err = bind_evtchn_to_irqhandler(info->evtchn, blkif_interrupt, 0,
- "blkif", info);
+ err = bind_evtchn_to_irqhandler(rinfo->evtchn, blkif_interrupt, 0,
+ "blkif", rinfo);
if (err <= 0) {
xenbus_dev_fatal(dev, err,
"bind_evtchn_to_irqhandler failed");
goto fail;
}
- info->irq = err;
+ rinfo->irq = err;
return 0;
fail:
@@ -1307,6 +1334,7 @@ static int talk_to_blkback(struct xenbus_device *dev,
int err, i;
unsigned int max_page_order = 0;
unsigned int ring_page_order = 0;
+ struct blkfront_ring_info *rinfo = &info->rinfo;
err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
"max-ring-page-order", "%u", &max_page_order);
@@ -1318,7 +1346,7 @@ static int talk_to_blkback(struct xenbus_device *dev,
}
/* Create shared ring, alloc event channel. */
- err = setup_blkring(dev, info);
+ err = setup_blkring(dev, rinfo);
if (err)
goto out;
@@ -1331,7 +1359,7 @@ again:
if (info->nr_ring_pages == 1) {
err = xenbus_printf(xbt, dev->nodename,
- "ring-ref", "%u", info->ring_ref[0]);
+ "ring-ref", "%u", rinfo->ring_ref[0]);
if (err) {
message = "writing ring-ref";
goto abort_transaction;
@@ -1349,7 +1377,7 @@ again:
snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
err = xenbus_printf(xbt, dev->nodename, ring_ref_name,
- "%u", info->ring_ref[i]);
+ "%u", rinfo->ring_ref[i]);
if (err) {
message = "writing ring-ref";
goto abort_transaction;
@@ -1357,7 +1385,7 @@ again:
}
}
err = xenbus_printf(xbt, dev->nodename,
- "event-channel", "%u", info->evtchn);
+ "event-channel", "%u", rinfo->evtchn);
if (err) {
message = "writing event-channel";
goto abort_transaction;
@@ -1383,8 +1411,8 @@ again:
}
for (i = 0; i < BLK_RING_SIZE(info); i++)
- info->shadow[i].req.u.rw.id = i+1;
- info->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
+ rinfo->shadow[i].req.u.rw.id = i+1;
+ rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
xenbus_switch_state(dev, XenbusStateInitialised);
return 0;
@@ -1410,6 +1438,7 @@ static int blkfront_probe(struct xenbus_device *dev,
{
int err, vdevice;
struct blkfront_info *info;
+ struct blkfront_ring_info *rinfo;
/* FIXME: Use dynamic device id if this is not set. */
err = xenbus_scanf(XBT_NIL, dev->nodename,
@@ -1459,15 +1488,18 @@ static int blkfront_probe(struct xenbus_device *dev,
return -ENOMEM;
}
+ rinfo = &info->rinfo;
+ INIT_LIST_HEAD(&rinfo->indirect_pages);
+ rinfo->dev_info = info;
+ INIT_WORK(&rinfo->work, blkif_restart_queue);
+
mutex_init(&info->mutex);
spin_lock_init(&info->io_lock);
info->xbdev = dev;
info->vdevice = vdevice;
INIT_LIST_HEAD(&info->grants);
- INIT_LIST_HEAD(&info->indirect_pages);
info->persistent_gnts_c = 0;
info->connected = BLKIF_STATE_DISCONNECTED;
- INIT_WORK(&info->work, blkif_restart_queue);
/* Front end dir is a number, which is used as the id. */
info->handle = simple_strtoul(strrchr(dev->nodename, '/')+1, NULL, 0);
@@ -1501,19 +1533,20 @@ static int blkif_recover(struct blkfront_info *info)
int pending, size;
struct split_bio *split_bio;
struct list_head requests;
+ struct blkfront_ring_info *rinfo = &info->rinfo;
/* Stage 1: Make a safe copy of the shadow state. */
- copy = kmemdup(info->shadow, sizeof(info->shadow),
+ copy = kmemdup(rinfo->shadow, sizeof(rinfo->shadow),
GFP_NOIO | __GFP_REPEAT | __GFP_HIGH);
if (!copy)
return -ENOMEM;
/* Stage 2: Set up free list. */
- memset(&info->shadow, 0, sizeof(info->shadow));
+ memset(&rinfo->shadow, 0, sizeof(rinfo->shadow));
for (i = 0; i < BLK_RING_SIZE(info); i++)
- info->shadow[i].req.u.rw.id = i+1;
- info->shadow_free = info->ring.req_prod_pvt;
- info->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
+ rinfo->shadow[i].req.u.rw.id = i+1;
+ rinfo->shadow_free = rinfo->ring.req_prod_pvt;
+ rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
rc = blkfront_gather_backend_features(info);
if (rc) {
@@ -1559,7 +1592,7 @@ static int blkif_recover(struct blkfront_info *info)
info->connected = BLKIF_STATE_CONNECTED;
/* Kick any other new requests queued since we resumed */
- kick_pending_request_queues(info);
+ kick_pending_request_queues(rinfo);
list_for_each_entry_safe(req, n, &requests, queuelist) {
/* Requeue pending requests (flush or discard) */
@@ -1693,17 +1726,18 @@ static void blkfront_setup_discard(struct blkfront_info *info)
info->feature_secdiscard = !!discard_secure;
}
-static int blkfront_setup_indirect(struct blkfront_info *info)
+static int blkfront_setup_indirect(struct blkfront_ring_info *rinfo)
{
unsigned int segs;
int err, i;
+ struct blkfront_info *info = rinfo->dev_info;
if (info->max_indirect_segments == 0)
segs = BLKIF_MAX_SEGMENTS_PER_REQUEST;
else
segs = info->max_indirect_segments;
- err = fill_grant_buffer(info, (segs + INDIRECT_GREFS(segs)) * BLK_RING_SIZE(info));
+ err = fill_grant_buffer(rinfo, (segs + INDIRECT_GREFS(segs)) * BLK_RING_SIZE(info));
if (err)
goto out_of_memory;
@@ -1715,31 +1749,31 @@ static int blkfront_setup_indirect(struct blkfront_info *info)
*/
int num = INDIRECT_GREFS(segs) * BLK_RING_SIZE(info);
- BUG_ON(!list_empty(&info->indirect_pages));
+ BUG_ON(!list_empty(&rinfo->indirect_pages));
for (i = 0; i < num; i++) {
struct page *indirect_page = alloc_page(GFP_NOIO);
if (!indirect_page)
goto out_of_memory;
- list_add(&indirect_page->lru, &info->indirect_pages);
+ list_add(&indirect_page->lru, &rinfo->indirect_pages);
}
}
for (i = 0; i < BLK_RING_SIZE(info); i++) {
- info->shadow[i].grants_used = kzalloc(
- sizeof(info->shadow[i].grants_used[0]) * segs,
+ rinfo->shadow[i].grants_used = kzalloc(
+ sizeof(rinfo->shadow[i].grants_used[0]) * segs,
GFP_NOIO);
- info->shadow[i].sg = kzalloc(sizeof(info->shadow[i].sg[0]) * segs, GFP_NOIO);
+ rinfo->shadow[i].sg = kzalloc(sizeof(rinfo->shadow[i].sg[0]) * segs, GFP_NOIO);
if (info->max_indirect_segments)
- info->shadow[i].indirect_grants = kzalloc(
- sizeof(info->shadow[i].indirect_grants[0]) *
+ rinfo->shadow[i].indirect_grants = kzalloc(
+ sizeof(rinfo->shadow[i].indirect_grants[0]) *
INDIRECT_GREFS(segs),
GFP_NOIO);
- if ((info->shadow[i].grants_used == NULL) ||
- (info->shadow[i].sg == NULL) ||
+ if ((rinfo->shadow[i].grants_used == NULL) ||
+ (rinfo->shadow[i].sg == NULL) ||
(info->max_indirect_segments &&
- (info->shadow[i].indirect_grants == NULL)))
+ (rinfo->shadow[i].indirect_grants == NULL)))
goto out_of_memory;
- sg_init_table(info->shadow[i].sg, segs);
+ sg_init_table(rinfo->shadow[i].sg, segs);
}
@@ -1747,16 +1781,16 @@ static int blkfront_setup_indirect(struct blkfront_info *info)
out_of_memory:
for (i = 0; i < BLK_RING_SIZE(info); i++) {
- kfree(info->shadow[i].grants_used);
- info->shadow[i].grants_used = NULL;
- kfree(info->shadow[i].sg);
- info->shadow[i].sg = NULL;
- kfree(info->shadow[i].indirect_grants);
- info->shadow[i].indirect_grants = NULL;
+ kfree(rinfo->shadow[i].grants_used);
+ rinfo->shadow[i].grants_used = NULL;
+ kfree(rinfo->shadow[i].sg);
+ rinfo->shadow[i].sg = NULL;
+ kfree(rinfo->shadow[i].indirect_grants);
+ rinfo->shadow[i].indirect_grants = NULL;
}
- if (!list_empty(&info->indirect_pages)) {
+ if (!list_empty(&rinfo->indirect_pages)) {
struct page *indirect_page, *n;
- list_for_each_entry_safe(indirect_page, n, &info->indirect_pages, lru) {
+ list_for_each_entry_safe(indirect_page, n, &rinfo->indirect_pages, lru) {
list_del(&indirect_page->lru);
__free_page(indirect_page);
}
@@ -1823,7 +1857,7 @@ static int blkfront_gather_backend_features(struct blkfront_info *info)
info->max_indirect_segments = min(indirect_segments,
xen_blkif_max_segments);
- return blkfront_setup_indirect(info);
+ return blkfront_setup_indirect(&info->rinfo);
}
/*
@@ -1837,6 +1871,7 @@ static void blkfront_connect(struct blkfront_info *info)
unsigned int physical_sector_size;
unsigned int binfo;
int err;
+ struct blkfront_ring_info *rinfo = &info->rinfo;
switch (info->connected) {
case BLKIF_STATE_CONNECTED:
@@ -1913,7 +1948,7 @@ static void blkfront_connect(struct blkfront_info *info)
/* Kick pending requests. */
spin_lock_irq(&info->io_lock);
info->connected = BLKIF_STATE_CONNECTED;
- kick_pending_request_queues(info);
+ kick_pending_request_queues(rinfo);
spin_unlock_irq(&info->io_lock);
add_disk(info->gd);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info
2015-11-02 4:21 ` [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info Bob Liu
@ 2015-11-02 4:49 ` kbuild test robot
2015-11-02 5:33 ` Bob Liu
0 siblings, 1 reply; 27+ messages in thread
From: kbuild test robot @ 2015-11-02 4:49 UTC (permalink / raw)
To: Bob Liu
Cc: kbuild-all, xen-devel, linux-kernel, roger.pau, konrad.wilk,
felipe.franciosi, axboe, avanzini.arianna, rafal.mielniczuk,
jonathan.davies, david.vrabel, Bob Liu
[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]
Hi Bob,
[auto build test ERROR on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Bob-Liu/xen-block-multi-hardware-queues-rings-support/20151102-122806
config: x86_64-allyesconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the linux-review/Bob-Liu/xen-block-multi-hardware-queues-rings-support/20151102-122806 HEAD b29fe44b095649f8faddc4474daba13199c1f5e0 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/block/xen-blkfront.c: In function 'blkif_queue_rq':
>> drivers/block/xen-blkfront.c:639:17: error: 'info' undeclared (first use in this function)
spin_lock_irq(&info->io_lock);
^
drivers/block/xen-blkfront.c:639:17: note: each undeclared identifier is reported only once for each function it appears in
vim +/info +639 drivers/block/xen-blkfront.c
907c3eb18 Bob Liu 2015-07-13 633 static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
907c3eb18 Bob Liu 2015-07-13 634 const struct blk_mq_queue_data *qd)
9f27ee595 Jeremy Fitzhardinge 2007-07-17 635 {
2a8974fd4 Bob Liu 2015-11-02 636 struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)hctx->driver_data;
9f27ee595 Jeremy Fitzhardinge 2007-07-17 637
907c3eb18 Bob Liu 2015-07-13 638 blk_mq_start_request(qd->rq);
907c3eb18 Bob Liu 2015-07-13 @639 spin_lock_irq(&info->io_lock);
2a8974fd4 Bob Liu 2015-11-02 640 if (RING_FULL(&rinfo->ring))
907c3eb18 Bob Liu 2015-07-13 641 goto out_busy;
9f27ee595 Jeremy Fitzhardinge 2007-07-17 642
:::::: The code at line 639 was first introduced by commit
:::::: 907c3eb18e0bd86ca12a9de80befe8e3647bac3e xen-blkfront: convert to blk-mq APIs
:::::: TO: Bob Liu <bob.liu@oracle.com>
:::::: CC: David Vrabel <david.vrabel@citrix.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 50361 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info
2015-11-02 4:49 ` kbuild test robot
@ 2015-11-02 5:33 ` Bob Liu
0 siblings, 0 replies; 27+ messages in thread
From: Bob Liu @ 2015-11-02 5:33 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, xen-devel, linux-kernel, roger.pau, konrad.wilk,
felipe.franciosi, axboe, avanzini.arianna, rafal.mielniczuk,
jonathan.davies, david.vrabel
On 11/02/2015 12:49 PM, kbuild test robot wrote:
> Hi Bob,
>
> [auto build test ERROR on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
>
> url: https://github.com/0day-ci/linux/commits/Bob-Liu/xen-block-multi-hardware-queues-rings-support/20151102-122806
> config: x86_64-allyesconfig (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> Note: the linux-review/Bob-Liu/xen-block-multi-hardware-queues-rings-support/20151102-122806 HEAD b29fe44b095649f8faddc4474daba13199c1f5e0 builds fine.
> It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
> drivers/block/xen-blkfront.c: In function 'blkif_queue_rq':
>>> drivers/block/xen-blkfront.c:639:17: error: 'info' undeclared (first use in this function)
> spin_lock_irq(&info->io_lock);
> ^
> drivers/block/xen-blkfront.c:639:17: note: each undeclared identifier is reported only once for each function it appears in
>
Sorry, I didn't do the compile-test after each patch.
Here is the fix and will update in next version.
[root@x4-4 linux]# git diff
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2a557e4..7face5e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -634,6 +634,7 @@ static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
const struct blk_mq_queue_data *qd)
{
struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)hctx->driver_data;
+ struct blkfront_info *info = rinfo->dev_info;
blk_mq_start_request(qd->rq);
spin_lock_irq(&info->io_lock);
[root@x4-4 linux]#
Thanks,
-Bob
> vim +/info +639 drivers/block/xen-blkfront.c
>
> 907c3eb18 Bob Liu 2015-07-13 633 static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
> 907c3eb18 Bob Liu 2015-07-13 634 const struct blk_mq_queue_data *qd)
> 9f27ee595 Jeremy Fitzhardinge 2007-07-17 635 {
> 2a8974fd4 Bob Liu 2015-11-02 636 struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)hctx->driver_data;
> 9f27ee595 Jeremy Fitzhardinge 2007-07-17 637
> 907c3eb18 Bob Liu 2015-07-13 638 blk_mq_start_request(qd->rq);
> 907c3eb18 Bob Liu 2015-07-13 @639 spin_lock_irq(&info->io_lock);
> 2a8974fd4 Bob Liu 2015-11-02 640 if (RING_FULL(&rinfo->ring))
> 907c3eb18 Bob Liu 2015-07-13 641 goto out_busy;
> 9f27ee595 Jeremy Fitzhardinge 2007-07-17 642
>
> :::::: The code at line 639 was first introduced by commit
> :::::: 907c3eb18e0bd86ca12a9de80befe8e3647bac3e xen-blkfront: convert to blk-mq APIs
>
> :::::: TO: Bob Liu <bob.liu@oracle.com>
> :::::: CC: David Vrabel <david.vrabel@citrix.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 03/10] xen/blkfront: pseudo support for multi hardware queues/rings
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
2015-11-02 4:21 ` [PATCH v4 01/10] xen/blkif: document blkif multi-queue/ring extension Bob Liu
2015-11-02 4:21 ` [PATCH v4 02/10] xen/blkfront: separate per ring information out of device info Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
[not found] ` <20151103194436.GE28527@char.us.oracle.com>
2015-11-02 4:21 ` [PATCH v4 04/10] xen/blkfront: split per device io_lock Bob Liu
` (7 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Preparatory patch for multiple hardware queues (rings). The number of
rings is unconditionally set to 1, larger number will be enabled in next
patch so as to make every single patch small and readable.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkfront.c | 327 +++++++++++++++++++++++++------------------
1 file changed, 188 insertions(+), 139 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2a557e4..eab78e7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -145,6 +145,7 @@ struct blkfront_info
int vdevice;
blkif_vdev_t handle;
enum blkif_state connected;
+ /* Number of pages per ring buffer */
unsigned int nr_ring_pages;
struct request_queue *rq;
struct list_head grants;
@@ -158,7 +159,8 @@ struct blkfront_info
unsigned int max_indirect_segments;
int is_ready;
struct blk_mq_tag_set tag_set;
- struct blkfront_ring_info rinfo;
+ struct blkfront_ring_info *rinfo;
+ unsigned int nr_rings;
};
static unsigned int nr_minors;
@@ -190,7 +192,7 @@ static DEFINE_SPINLOCK(minor_lock);
((_segs + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
static int blkfront_setup_indirect(struct blkfront_ring_info *rinfo);
-static int blkfront_gather_backend_features(struct blkfront_info *info);
+static void blkfront_gather_backend_features(struct blkfront_info *info);
static int get_id_from_freelist(struct blkfront_ring_info *rinfo)
{
@@ -443,12 +445,13 @@ static int blkif_queue_request(struct request *req, struct blkfront_ring_info *r
*/
max_grefs += INDIRECT_GREFS(req->nr_phys_segments);
- /* Check if we have enough grants to allocate a requests */
- if (info->persistent_gnts_c < max_grefs) {
+ /* Check if we have enough grants to allocate a requests, we have to
+ * reserve 'max_grefs' grants because persistent grants are shared by all
+ * rings */
+ if (0 < max_grefs) {
new_persistent_gnts = 1;
if (gnttab_alloc_grant_references(
- max_grefs - info->persistent_gnts_c,
- &gref_head) < 0) {
+ max_grefs, &gref_head) < 0) {
gnttab_request_free_callback(
&rinfo->callback,
blkif_restart_queue_callback,
@@ -665,7 +668,7 @@ static int blk_mq_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
{
struct blkfront_info *info = (struct blkfront_info *)data;
- hctx->driver_data = &info->rinfo;
+ hctx->driver_data = &info->rinfo[index];
return 0;
}
@@ -924,8 +927,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
static void xlvbd_release_gendisk(struct blkfront_info *info)
{
- unsigned int minor, nr_minors;
- struct blkfront_ring_info *rinfo = &info->rinfo;
+ unsigned int minor, nr_minors, i;
if (info->rq == NULL)
return;
@@ -933,11 +935,15 @@ static void xlvbd_release_gendisk(struct blkfront_info *info)
/* No more blkif_request(). */
blk_mq_stop_hw_queues(info->rq);
- /* No more gnttab callback work. */
- gnttab_cancel_free_callback(&rinfo->callback);
+ for (i = 0; i < info->nr_rings; i++) {
+ struct blkfront_ring_info *rinfo = &info->rinfo[i];
- /* Flush gnttab callback work. Must be done with no locks held. */
- flush_work(&rinfo->work);
+ /* No more gnttab callback work. */
+ gnttab_cancel_free_callback(&rinfo->callback);
+
+ /* Flush gnttab callback work. Must be done with no locks held. */
+ flush_work(&rinfo->work);
+ }
del_gendisk(info->gd);
@@ -970,37 +976,11 @@ static void blkif_restart_queue(struct work_struct *work)
spin_unlock_irq(&rinfo->dev_info->io_lock);
}
-static void blkif_free(struct blkfront_info *info, int suspend)
+static void blkif_free_ring(struct blkfront_ring_info *rinfo)
{
struct grant *persistent_gnt;
- struct grant *n;
+ struct blkfront_info *info = rinfo->dev_info;
int i, j, segs;
- struct blkfront_ring_info *rinfo = &info->rinfo;
-
- /* Prevent new requests being issued until we fix things up. */
- spin_lock_irq(&info->io_lock);
- info->connected = suspend ?
- BLKIF_STATE_SUSPENDED : BLKIF_STATE_DISCONNECTED;
- /* No more blkif_request(). */
- if (info->rq)
- blk_mq_stop_hw_queues(info->rq);
-
- /* Remove all persistent grants */
- if (!list_empty(&info->grants)) {
- list_for_each_entry_safe(persistent_gnt, n,
- &info->grants, node) {
- list_del(&persistent_gnt->node);
- if (persistent_gnt->gref != GRANT_INVALID_REF) {
- gnttab_end_foreign_access(persistent_gnt->gref,
- 0, 0UL);
- info->persistent_gnts_c--;
- }
- if (info->feature_persistent)
- __free_page(pfn_to_page(persistent_gnt->pfn));
- kfree(persistent_gnt);
- }
- }
- BUG_ON(info->persistent_gnts_c != 0);
/*
* Remove indirect pages, this only happens when using indirect
@@ -1060,7 +1040,6 @@ free_shadow:
/* No more gnttab callback work. */
gnttab_cancel_free_callback(&rinfo->callback);
- spin_unlock_irq(&info->io_lock);
/* Flush gnttab callback work. Must be done with no locks held. */
flush_work(&rinfo->work);
@@ -1078,7 +1057,41 @@ free_shadow:
if (rinfo->irq)
unbind_from_irqhandler(rinfo->irq, rinfo);
rinfo->evtchn = rinfo->irq = 0;
+}
+static void blkif_free(struct blkfront_info *info, int suspend)
+{
+ struct grant *persistent_gnt, *n;
+ int i;
+
+ /* Prevent new requests being issued until we fix things up. */
+ spin_lock_irq(&info->io_lock);
+ info->connected = suspend ?
+ BLKIF_STATE_SUSPENDED : BLKIF_STATE_DISCONNECTED;
+ /* No more blkif_request(). */
+ if (info->rq)
+ blk_mq_stop_hw_queues(info->rq);
+
+ /* Remove all persistent grants */
+ if (!list_empty(&info->grants)) {
+ list_for_each_entry_safe(persistent_gnt, n,
+ &info->grants, node) {
+ list_del(&persistent_gnt->node);
+ if (persistent_gnt->gref != GRANT_INVALID_REF) {
+ gnttab_end_foreign_access(persistent_gnt->gref,
+ 0, 0UL);
+ info->persistent_gnts_c--;
+ }
+ if (info->feature_persistent)
+ __free_page(pfn_to_page(persistent_gnt->pfn));
+ kfree(persistent_gnt);
+ }
+ }
+ BUG_ON(info->persistent_gnts_c != 0);
+
+ for (i = 0; i < info->nr_rings; i++)
+ blkif_free_ring(&info->rinfo[i]);
+ spin_unlock_irq(&info->io_lock);
}
static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *rinfo,
@@ -1334,7 +1347,7 @@ static int talk_to_blkback(struct xenbus_device *dev,
int err, i;
unsigned int max_page_order = 0;
unsigned int ring_page_order = 0;
- struct blkfront_ring_info *rinfo = &info->rinfo;
+ struct blkfront_ring_info *rinfo;
err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
"max-ring-page-order", "%u", &max_page_order);
@@ -1345,10 +1358,13 @@ static int talk_to_blkback(struct xenbus_device *dev,
info->nr_ring_pages = 1 << ring_page_order;
}
- /* Create shared ring, alloc event channel. */
- err = setup_blkring(dev, rinfo);
- if (err)
- goto out;
+ for (i = 0; i < info->nr_rings; i++) {
+ rinfo = &info->rinfo[i];
+ /* Create shared ring, alloc event channel. */
+ err = setup_blkring(dev, rinfo);
+ if (err)
+ goto destroy_blkring;
+ }
again:
err = xenbus_transaction_start(&xbt);
@@ -1357,37 +1373,43 @@ again:
goto destroy_blkring;
}
- if (info->nr_ring_pages == 1) {
- err = xenbus_printf(xbt, dev->nodename,
- "ring-ref", "%u", rinfo->ring_ref[0]);
- if (err) {
- message = "writing ring-ref";
- goto abort_transaction;
- }
- } else {
- err = xenbus_printf(xbt, dev->nodename,
- "ring-page-order", "%u", ring_page_order);
- if (err) {
- message = "writing ring-page-order";
- goto abort_transaction;
- }
-
- for (i = 0; i < info->nr_ring_pages; i++) {
- char ring_ref_name[RINGREF_NAME_LEN];
-
- snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
- err = xenbus_printf(xbt, dev->nodename, ring_ref_name,
- "%u", rinfo->ring_ref[i]);
+ if (info->nr_rings == 1) {
+ rinfo = &info->rinfo[0];
+ if (info->nr_ring_pages == 1) {
+ err = xenbus_printf(xbt, dev->nodename,
+ "ring-ref", "%u", rinfo->ring_ref[0]);
if (err) {
message = "writing ring-ref";
goto abort_transaction;
}
+ } else {
+ err = xenbus_printf(xbt, dev->nodename,
+ "ring-page-order", "%u", ring_page_order);
+ if (err) {
+ message = "writing ring-page-order";
+ goto abort_transaction;
+ }
+
+ for (i = 0; i < info->nr_ring_pages; i++) {
+ char ring_ref_name[RINGREF_NAME_LEN];
+
+ snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
+ err = xenbus_printf(xbt, dev->nodename, ring_ref_name,
+ "%u", rinfo->ring_ref[i]);
+ if (err) {
+ message = "writing ring-ref";
+ goto abort_transaction;
+ }
+ }
}
- }
- err = xenbus_printf(xbt, dev->nodename,
- "event-channel", "%u", rinfo->evtchn);
- if (err) {
- message = "writing event-channel";
+ err = xenbus_printf(xbt, dev->nodename,
+ "event-channel", "%u", rinfo->evtchn);
+ if (err) {
+ message = "writing event-channel";
+ goto abort_transaction;
+ }
+ } else {
+ /* Not supported at this stage */
goto abort_transaction;
}
err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
@@ -1410,9 +1432,14 @@ again:
goto destroy_blkring;
}
- for (i = 0; i < BLK_RING_SIZE(info); i++)
- rinfo->shadow[i].req.u.rw.id = i+1;
- rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
+ for (i = 0; i < info->nr_rings; i++) {
+ int j;
+ rinfo = &info->rinfo[i];
+
+ for (j = 0; j < BLK_RING_SIZE(info); j++)
+ rinfo->shadow[j].req.u.rw.id = j + 1;
+ rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
+ }
xenbus_switch_state(dev, XenbusStateInitialised);
return 0;
@@ -1423,7 +1450,7 @@ again:
xenbus_dev_fatal(dev, err, "%s", message);
destroy_blkring:
blkif_free(info, 0);
- out:
+
return err;
}
@@ -1436,9 +1463,8 @@ again:
static int blkfront_probe(struct xenbus_device *dev,
const struct xenbus_device_id *id)
{
- int err, vdevice;
+ int err, vdevice, r_index;
struct blkfront_info *info;
- struct blkfront_ring_info *rinfo;
/* FIXME: Use dynamic device id if this is not set. */
err = xenbus_scanf(XBT_NIL, dev->nodename,
@@ -1488,10 +1514,22 @@ static int blkfront_probe(struct xenbus_device *dev,
return -ENOMEM;
}
- rinfo = &info->rinfo;
- INIT_LIST_HEAD(&rinfo->indirect_pages);
- rinfo->dev_info = info;
- INIT_WORK(&rinfo->work, blkif_restart_queue);
+ info->nr_rings = 1;
+ info->rinfo = kzalloc(sizeof(struct blkfront_ring_info) * info->nr_rings, GFP_KERNEL);
+ if (!info->rinfo) {
+ xenbus_dev_fatal(dev, -ENOMEM, "allocating ring_info structure");
+ kfree(info);
+ return -ENOMEM;
+ }
+
+ for (r_index = 0; r_index < info->nr_rings; r_index++) {
+ struct blkfront_ring_info *rinfo;
+
+ rinfo = &info->rinfo[r_index];
+ INIT_LIST_HEAD(&rinfo->indirect_pages);
+ rinfo->dev_info = info;
+ INIT_WORK(&rinfo->work, blkif_restart_queue);
+ }
mutex_init(&info->mutex);
spin_lock_init(&info->io_lock);
@@ -1523,7 +1561,7 @@ static void split_bio_end(struct bio *bio)
static int blkif_recover(struct blkfront_info *info)
{
- int i;
+ int i, r_index;
struct request *req, *n;
struct blk_shadow *copy;
int rc;
@@ -1533,57 +1571,62 @@ static int blkif_recover(struct blkfront_info *info)
int pending, size;
struct split_bio *split_bio;
struct list_head requests;
- struct blkfront_ring_info *rinfo = &info->rinfo;
-
- /* Stage 1: Make a safe copy of the shadow state. */
- copy = kmemdup(rinfo->shadow, sizeof(rinfo->shadow),
- GFP_NOIO | __GFP_REPEAT | __GFP_HIGH);
- if (!copy)
- return -ENOMEM;
-
- /* Stage 2: Set up free list. */
- memset(&rinfo->shadow, 0, sizeof(rinfo->shadow));
- for (i = 0; i < BLK_RING_SIZE(info); i++)
- rinfo->shadow[i].req.u.rw.id = i+1;
- rinfo->shadow_free = rinfo->ring.req_prod_pvt;
- rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
-
- rc = blkfront_gather_backend_features(info);
- if (rc) {
- kfree(copy);
- return rc;
- }
+ blkfront_gather_backend_features(info);
segs = info->max_indirect_segments ? : BLKIF_MAX_SEGMENTS_PER_REQUEST;
blk_queue_max_segments(info->rq, segs);
bio_list_init(&bio_list);
INIT_LIST_HEAD(&requests);
- for (i = 0; i < BLK_RING_SIZE(info); i++) {
- /* Not in use? */
- if (!copy[i].request)
- continue;
- /*
- * Get the bios in the request so we can re-queue them.
- */
- if (copy[i].request->cmd_flags &
- (REQ_FLUSH | REQ_FUA | REQ_DISCARD | REQ_SECURE)) {
+ for (r_index = 0; r_index < info->nr_rings; r_index++) {
+ struct blkfront_ring_info *rinfo;
+
+ rinfo = &info->rinfo[r_index];
+ /* Stage 1: Make a safe copy of the shadow state. */
+ copy = kmemdup(rinfo->shadow, sizeof(rinfo->shadow),
+ GFP_NOIO | __GFP_REPEAT | __GFP_HIGH);
+ if (!copy)
+ return -ENOMEM;
+
+ /* Stage 2: Set up free list. */
+ memset(&rinfo->shadow, 0, sizeof(rinfo->shadow));
+ for (i = 0; i < BLK_RING_SIZE(info); i++)
+ rinfo->shadow[i].req.u.rw.id = i+1;
+ rinfo->shadow_free = rinfo->ring.req_prod_pvt;
+ rinfo->shadow[BLK_RING_SIZE(info)-1].req.u.rw.id = 0x0fffffff;
+
+ rc = blkfront_setup_indirect(rinfo);
+ if (rc) {
+ kfree(copy);
+ return rc;
+ }
+
+ for (i = 0; i < BLK_RING_SIZE(info); i++) {
+ /* Not in use? */
+ if (!copy[i].request)
+ continue;
+
/*
- * Flush operations don't contain bios, so
- * we need to requeue the whole request
+ * Get the bios in the request so we can re-queue them.
*/
- list_add(©[i].request->queuelist, &requests);
- continue;
+ if (copy[i].request->cmd_flags &
+ (REQ_FLUSH | REQ_FUA | REQ_DISCARD | REQ_SECURE)) {
+ /*
+ * Flush operations don't contain bios, so
+ * we need to requeue the whole request
+ */
+ list_add(©[i].request->queuelist, &requests);
+ continue;
+ }
+ merge_bio.head = copy[i].request->bio;
+ merge_bio.tail = copy[i].request->biotail;
+ bio_list_merge(&bio_list, &merge_bio);
+ copy[i].request->bio = NULL;
+ blk_end_request_all(copy[i].request, 0);
}
- merge_bio.head = copy[i].request->bio;
- merge_bio.tail = copy[i].request->biotail;
- bio_list_merge(&bio_list, &merge_bio);
- copy[i].request->bio = NULL;
- blk_end_request_all(copy[i].request, 0);
- }
-
- kfree(copy);
+ kfree(copy);
+ }
xenbus_switch_state(info->xbdev, XenbusStateConnected);
spin_lock_irq(&info->io_lock);
@@ -1591,8 +1634,13 @@ static int blkif_recover(struct blkfront_info *info)
/* Now safe for us to use the shared ring */
info->connected = BLKIF_STATE_CONNECTED;
- /* Kick any other new requests queued since we resumed */
- kick_pending_request_queues(rinfo);
+ for (r_index = 0; r_index < info->nr_rings; r_index++) {
+ struct blkfront_ring_info *rinfo;
+
+ rinfo = &info->rinfo[r_index];
+ /* Kick any other new requests queued since we resumed */
+ kick_pending_request_queues(rinfo);
+ }
list_for_each_entry_safe(req, n, &requests, queuelist) {
/* Requeue pending requests (flush or discard) */
@@ -1801,7 +1849,7 @@ out_of_memory:
/*
* Gather all backend feature-*
*/
-static int blkfront_gather_backend_features(struct blkfront_info *info)
+static void blkfront_gather_backend_features(struct blkfront_info *info)
{
int err;
int barrier, flush, discard, persistent;
@@ -1856,8 +1904,6 @@ static int blkfront_gather_backend_features(struct blkfront_info *info)
else
info->max_indirect_segments = min(indirect_segments,
xen_blkif_max_segments);
-
- return blkfront_setup_indirect(&info->rinfo);
}
/*
@@ -1870,8 +1916,7 @@ static void blkfront_connect(struct blkfront_info *info)
unsigned long sector_size;
unsigned int physical_sector_size;
unsigned int binfo;
- int err;
- struct blkfront_ring_info *rinfo = &info->rinfo;
+ int err, i;
switch (info->connected) {
case BLKIF_STATE_CONNECTED:
@@ -1928,11 +1973,14 @@ static void blkfront_connect(struct blkfront_info *info)
if (err != 1)
physical_sector_size = sector_size;
- err = blkfront_gather_backend_features(info);
- if (err) {
- xenbus_dev_fatal(info->xbdev, err, "setup_indirect at %s",
- info->xbdev->otherend);
- return;
+ blkfront_gather_backend_features(info);
+ for (i = 0; i < info->nr_rings; i++) {
+ err = blkfront_setup_indirect(&info->rinfo[i]);
+ if (err) {
+ xenbus_dev_fatal(info->xbdev, err, "setup_indirect at %s",
+ info->xbdev->otherend);
+ blkif_free(info, 0);
+ }
}
err = xlvbd_alloc_gendisk(sectors, info, binfo, sector_size,
@@ -1948,7 +1996,8 @@ static void blkfront_connect(struct blkfront_info *info)
/* Kick pending requests. */
spin_lock_irq(&info->io_lock);
info->connected = BLKIF_STATE_CONNECTED;
- kick_pending_request_queues(rinfo);
+ for (i = 0; i < info->nr_rings; i++)
+ kick_pending_request_queues(&info->rinfo[i]);
spin_unlock_irq(&info->io_lock);
add_disk(info->gd);
@@ -2111,7 +2160,7 @@ static void blkif_release(struct gendisk *disk, fmode_t mode)
dev_info(disk_to_dev(bdev->bd_disk), "releasing disk\n");
xlvbd_release_gendisk(info);
xenbus_frontend_closed(info->xbdev);
- }
+ }
mutex_unlock(&info->mutex);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 04/10] xen/blkfront: split per device io_lock
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (2 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 03/10] xen/blkfront: pseudo support for multi hardware queues/rings Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
[not found] ` <20151103200902.GF28527@char.us.oracle.com>
2015-11-02 4:21 ` [PATCH v4 05/10] xen/blkfront: negotiate number of queues/rings to be used with backend Bob Liu
` (6 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
The per device io_lock became a coarser grained lock after multi-queues/rings
was introduced, this patch introduced a fine-grained ring_lock for each ring.
The old io_lock was renamed to dev_lock and only protect the ->grants list
which is shared by all rings.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkfront.c | 57 ++++++++++++++++++++++++++------------------
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index eab78e7..8cc5995 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -121,6 +121,7 @@ MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the
*/
struct blkfront_ring_info {
struct blkif_front_ring ring;
+ spinlock_t ring_lock;
unsigned int ring_ref[XENBUS_MAX_RING_PAGES];
unsigned int evtchn, irq;
struct work_struct work;
@@ -138,7 +139,8 @@ struct blkfront_ring_info {
*/
struct blkfront_info
{
- spinlock_t io_lock;
+ /* Lock to proect info->grants list shared by multi rings */
+ spinlock_t dev_lock;
struct mutex mutex;
struct xenbus_device *xbdev;
struct gendisk *gd;
@@ -224,6 +226,7 @@ static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
struct grant *gnt_list_entry, *n;
int i = 0;
+ spin_lock_irq(&info->dev_lock);
while(i < num) {
gnt_list_entry = kzalloc(sizeof(struct grant), GFP_NOIO);
if (!gnt_list_entry)
@@ -242,6 +245,7 @@ static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
list_add(&gnt_list_entry->node, &info->grants);
i++;
}
+ spin_unlock_irq(&info->dev_lock);
return 0;
@@ -254,6 +258,7 @@ out_of_memory:
kfree(gnt_list_entry);
i--;
}
+ spin_unlock_irq(&info->dev_lock);
BUG_ON(i != 0);
return -ENOMEM;
}
@@ -265,6 +270,7 @@ static struct grant *get_grant(grant_ref_t *gref_head,
struct grant *gnt_list_entry;
unsigned long buffer_gfn;
+ spin_lock(&info->dev_lock);
BUG_ON(list_empty(&info->grants));
gnt_list_entry = list_first_entry(&info->grants, struct grant,
node);
@@ -272,8 +278,10 @@ static struct grant *get_grant(grant_ref_t *gref_head,
if (gnt_list_entry->gref != GRANT_INVALID_REF) {
info->persistent_gnts_c--;
+ spin_unlock(&info->dev_lock);
return gnt_list_entry;
}
+ spin_unlock(&info->dev_lock);
/* Assign a gref to this page */
gnt_list_entry->gref = gnttab_claim_grant_reference(gref_head);
@@ -639,7 +647,7 @@ static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)hctx->driver_data;
blk_mq_start_request(qd->rq);
- spin_lock_irq(&info->io_lock);
+ spin_lock_irq(&rinfo->ring_lock);
if (RING_FULL(&rinfo->ring))
goto out_busy;
@@ -650,15 +658,15 @@ static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
goto out_busy;
flush_requests(rinfo);
- spin_unlock_irq(&info->io_lock);
+ spin_unlock_irq(&rinfo->ring_lock);
return BLK_MQ_RQ_QUEUE_OK;
out_err:
- spin_unlock_irq(&info->io_lock);
+ spin_unlock_irq(&rinfo->ring_lock);
return BLK_MQ_RQ_QUEUE_ERROR;
out_busy:
- spin_unlock_irq(&info->io_lock);
+ spin_unlock_irq(&rinfo->ring_lock);
blk_mq_stop_hw_queue(hctx);
return BLK_MQ_RQ_QUEUE_BUSY;
}
@@ -959,21 +967,22 @@ static void xlvbd_release_gendisk(struct blkfront_info *info)
info->gd = NULL;
}
-/* Must be called with io_lock holded */
static void kick_pending_request_queues(struct blkfront_ring_info *rinfo)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&rinfo->ring_lock, flags);
if (!RING_FULL(&rinfo->ring))
blk_mq_start_stopped_hw_queues(rinfo->dev_info->rq, true);
+ spin_unlock_irqrestore(&rinfo->ring_lock, flags);
}
static void blkif_restart_queue(struct work_struct *work)
{
struct blkfront_ring_info *rinfo = container_of(work, struct blkfront_ring_info, work);
- spin_lock_irq(&rinfo->dev_info->io_lock);
if (rinfo->dev_info->connected == BLKIF_STATE_CONNECTED)
kick_pending_request_queues(rinfo);
- spin_unlock_irq(&rinfo->dev_info->io_lock);
}
static void blkif_free_ring(struct blkfront_ring_info *rinfo)
@@ -1065,7 +1074,7 @@ static void blkif_free(struct blkfront_info *info, int suspend)
int i;
/* Prevent new requests being issued until we fix things up. */
- spin_lock_irq(&info->io_lock);
+ spin_lock_irq(&info->dev_lock);
info->connected = suspend ?
BLKIF_STATE_SUSPENDED : BLKIF_STATE_DISCONNECTED;
/* No more blkif_request(). */
@@ -1091,7 +1100,7 @@ static void blkif_free(struct blkfront_info *info, int suspend)
for (i = 0; i < info->nr_rings; i++)
blkif_free_ring(&info->rinfo[i]);
- spin_unlock_irq(&info->io_lock);
+ spin_unlock_irq(&info->dev_lock);
}
static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *rinfo,
@@ -1103,6 +1112,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
void *shared_data;
int nseg;
struct blkfront_info *info = rinfo->dev_info;
+ unsigned long flags;
nseg = s->req.operation == BLKIF_OP_INDIRECT ?
s->req.u.indirect.nr_segments : s->req.u.rw.nr_segments;
@@ -1120,6 +1130,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
kunmap_atomic(shared_data);
}
}
+
/* Add the persistent grant into the list of free grants */
for (i = 0; i < nseg; i++) {
if (gnttab_query_foreign_access(s->grants_used[i]->gref)) {
@@ -1132,8 +1143,10 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
if (!info->feature_persistent)
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
s->grants_used[i]->gref);
+ spin_lock_irqsave(&info->dev_lock, flags);
list_add(&s->grants_used[i]->node, &info->grants);
info->persistent_gnts_c++;
+ spin_unlock_irqrestore(&info->dev_lock, flags);
} else {
/*
* If the grant is not mapped by the backend we end the
@@ -1143,7 +1156,9 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
*/
gnttab_end_foreign_access(s->grants_used[i]->gref, 0, 0UL);
s->grants_used[i]->gref = GRANT_INVALID_REF;
+ spin_lock_irqsave(&info->dev_lock, flags);
list_add_tail(&s->grants_used[i]->node, &info->grants);
+ spin_unlock_irqrestore(&info->dev_lock, flags);
}
}
if (s->req.operation == BLKIF_OP_INDIRECT) {
@@ -1152,8 +1167,10 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
if (!info->feature_persistent)
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
s->indirect_grants[i]->gref);
+ spin_lock_irqsave(&info->dev_lock, flags);
list_add(&s->indirect_grants[i]->node, &info->grants);
info->persistent_gnts_c++;
+ spin_unlock_irqrestore(&info->dev_lock, flags);
} else {
struct page *indirect_page;
@@ -1162,12 +1179,14 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
* Add the used indirect page back to the list of
* available pages for indirect grefs.
*/
+ spin_lock_irqsave(&info->dev_lock, flags);
if (!info->feature_persistent) {
indirect_page = pfn_to_page(s->indirect_grants[i]->pfn);
list_add(&indirect_page->lru, &rinfo->indirect_pages);
}
s->indirect_grants[i]->gref = GRANT_INVALID_REF;
list_add_tail(&s->indirect_grants[i]->node, &info->grants);
+ spin_unlock_irqrestore(&info->dev_lock, flags);
}
}
}
@@ -1183,13 +1202,10 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
struct blkfront_info *info = rinfo->dev_info;
int error;
- spin_lock_irqsave(&info->io_lock, flags);
-
- if (unlikely(info->connected != BLKIF_STATE_CONNECTED)) {
- spin_unlock_irqrestore(&info->io_lock, flags);
+ if (unlikely(info->connected != BLKIF_STATE_CONNECTED))
return IRQ_HANDLED;
- }
+ spin_lock_irqsave(&rinfo->ring_lock, flags);
again:
rp = rinfo->ring.sring->rsp_prod;
rmb(); /* Ensure we see queued responses up to 'rp'. */
@@ -1280,10 +1296,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
} else
rinfo->ring.sring->rsp_event = i + 1;
+ spin_unlock_irqrestore(&rinfo->ring_lock, flags);
kick_pending_request_queues(rinfo);
- spin_unlock_irqrestore(&info->io_lock, flags);
-
return IRQ_HANDLED;
}
@@ -1529,10 +1544,11 @@ static int blkfront_probe(struct xenbus_device *dev,
INIT_LIST_HEAD(&rinfo->indirect_pages);
rinfo->dev_info = info;
INIT_WORK(&rinfo->work, blkif_restart_queue);
+ spin_lock_init(&rinfo->ring_lock);
}
mutex_init(&info->mutex);
- spin_lock_init(&info->io_lock);
+ spin_lock_init(&info->dev_lock);
info->xbdev = dev;
info->vdevice = vdevice;
INIT_LIST_HEAD(&info->grants);
@@ -1629,8 +1645,6 @@ static int blkif_recover(struct blkfront_info *info)
}
xenbus_switch_state(info->xbdev, XenbusStateConnected);
- spin_lock_irq(&info->io_lock);
-
/* Now safe for us to use the shared ring */
info->connected = BLKIF_STATE_CONNECTED;
@@ -1648,7 +1662,6 @@ static int blkif_recover(struct blkfront_info *info)
BUG_ON(req->nr_phys_segments > segs);
blk_mq_requeue_request(req);
}
- spin_unlock_irq(&info->io_lock);
blk_mq_kick_requeue_list(info->rq);
while ((bio = bio_list_pop(&bio_list)) != NULL) {
@@ -1994,11 +2007,9 @@ static void blkfront_connect(struct blkfront_info *info)
xenbus_switch_state(info->xbdev, XenbusStateConnected);
/* Kick pending requests. */
- spin_lock_irq(&info->io_lock);
info->connected = BLKIF_STATE_CONNECTED;
for (i = 0; i < info->nr_rings; i++)
kick_pending_request_queues(&info->rinfo[i]);
- spin_unlock_irq(&info->io_lock);
add_disk(info->gd);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 05/10] xen/blkfront: negotiate number of queues/rings to be used with backend
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (3 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 04/10] xen/blkfront: split per device io_lock Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
[not found] ` <20151103204029.GH28527@char.us.oracle.com>
2015-11-02 4:21 ` [PATCH v4 06/10] xen/blkback: separate ring information out of struct xen_blkif Bob Liu
` (5 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
The number of hardware queues for xen/blkfront is set by parameter
'max_queues'(default 4), while the max value xen/blkback supported is notified
through xenstore("multi-queue-max-queues").
The negotiated number is the smaller one and would be written back to xenstore
as "multi-queue-num-queues", blkback need to read this negotiated number.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkfront.c | 166 +++++++++++++++++++++++++++++++------------
1 file changed, 120 insertions(+), 46 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8cc5995..23096d7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -98,6 +98,10 @@ static unsigned int xen_blkif_max_segments = 32;
module_param_named(max, xen_blkif_max_segments, int, S_IRUGO);
MODULE_PARM_DESC(max, "Maximum amount of segments in indirect requests (default is 32)");
+static unsigned int xen_blkif_max_queues = 4;
+module_param_named(max_queues, xen_blkif_max_queues, uint, S_IRUGO);
+MODULE_PARM_DESC(max_queues, "Maximum number of hardware queues/rings used per virtual disk");
+
/*
* Maximum order of pages to be used for the shared ring between front and
* backend, 4KB page granularity is used.
@@ -113,6 +117,7 @@ MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the
* characters are enough. Define to 20 to keep consist with backend.
*/
#define RINGREF_NAME_LEN (20)
+#define QUEUE_NAME_LEN (12)
/*
* Per-ring info.
@@ -695,7 +700,7 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
memset(&info->tag_set, 0, sizeof(info->tag_set));
info->tag_set.ops = &blkfront_mq_ops;
- info->tag_set.nr_hw_queues = 1;
+ info->tag_set.nr_hw_queues = info->nr_rings;
info->tag_set.queue_depth = BLK_RING_SIZE(info);
info->tag_set.numa_node = NUMA_NO_NODE;
info->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
@@ -1352,6 +1357,51 @@ fail:
return err;
}
+static int write_per_ring_nodes(struct xenbus_transaction xbt,
+ struct blkfront_ring_info *rinfo, const char *dir)
+{
+ int err, i;
+ const char *message = NULL;
+ struct blkfront_info *info = rinfo->dev_info;
+
+ if (info->nr_ring_pages == 1) {
+ err = xenbus_printf(xbt, dir, "ring-ref", "%u", rinfo->ring_ref[0]);
+ if (err) {
+ message = "writing ring-ref";
+ goto abort_transaction;
+ }
+ pr_info("%s: write ring-ref:%d\n", dir, rinfo->ring_ref[0]);
+ } else {
+ for (i = 0; i < info->nr_ring_pages; i++) {
+ char ring_ref_name[RINGREF_NAME_LEN];
+
+ snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
+ err = xenbus_printf(xbt, dir, ring_ref_name,
+ "%u", rinfo->ring_ref[i]);
+ if (err) {
+ message = "writing ring-ref";
+ goto abort_transaction;
+ }
+ pr_info("%s: write ring-ref:%d\n", dir, rinfo->ring_ref[i]);
+ }
+ }
+
+ err = xenbus_printf(xbt, dir, "event-channel", "%u", rinfo->evtchn);
+ if (err) {
+ message = "writing event-channel";
+ goto abort_transaction;
+ }
+ pr_info("%s: write event-channel:%d\n", dir, rinfo->evtchn);
+
+ return 0;
+
+abort_transaction:
+ xenbus_transaction_end(xbt, 1);
+ if (message)
+ xenbus_dev_fatal(info->xbdev, err, "%s", message);
+
+ return err;
+}
/* Common code used when first setting up, and when resuming. */
static int talk_to_blkback(struct xenbus_device *dev,
@@ -1362,7 +1412,6 @@ static int talk_to_blkback(struct xenbus_device *dev,
int err, i;
unsigned int max_page_order = 0;
unsigned int ring_page_order = 0;
- struct blkfront_ring_info *rinfo;
err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
"max-ring-page-order", "%u", &max_page_order);
@@ -1374,7 +1423,8 @@ static int talk_to_blkback(struct xenbus_device *dev,
}
for (i = 0; i < info->nr_rings; i++) {
- rinfo = &info->rinfo[i];
+ struct blkfront_ring_info *rinfo = &info->rinfo[i];
+
/* Create shared ring, alloc event channel. */
err = setup_blkring(dev, rinfo);
if (err)
@@ -1388,45 +1438,51 @@ again:
goto destroy_blkring;
}
- if (info->nr_rings == 1) {
- rinfo = &info->rinfo[0];
- if (info->nr_ring_pages == 1) {
- err = xenbus_printf(xbt, dev->nodename,
- "ring-ref", "%u", rinfo->ring_ref[0]);
- if (err) {
- message = "writing ring-ref";
- goto abort_transaction;
- }
- } else {
- err = xenbus_printf(xbt, dev->nodename,
- "ring-page-order", "%u", ring_page_order);
- if (err) {
- message = "writing ring-page-order";
- goto abort_transaction;
- }
+ if (info->nr_ring_pages > 1) {
+ err = xenbus_printf(xbt, dev->nodename, "ring-page-order", "%u",
+ ring_page_order);
+ if (err) {
+ message = "writing ring-page-order";
+ goto abort_transaction;
+ }
+ }
- for (i = 0; i < info->nr_ring_pages; i++) {
- char ring_ref_name[RINGREF_NAME_LEN];
+ /* We already got the number of queues/rings in _probe */
+ if (info->nr_rings == 1) {
+ err = write_per_ring_nodes(xbt, &info->rinfo[0], dev->nodename);
+ if (err)
+ goto destroy_blkring;
+ } else {
+ char *path;
+ size_t pathsize;
- snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
- err = xenbus_printf(xbt, dev->nodename, ring_ref_name,
- "%u", rinfo->ring_ref[i]);
- if (err) {
- message = "writing ring-ref";
- goto abort_transaction;
- }
- }
- }
- err = xenbus_printf(xbt, dev->nodename,
- "event-channel", "%u", rinfo->evtchn);
+ err = xenbus_printf(xbt, dev->nodename, "multi-queue-num-queues", "%u",
+ info->nr_rings);
if (err) {
- message = "writing event-channel";
+ message = "writing multi-queue-num-queues";
goto abort_transaction;
}
- } else {
- /* Not supported at this stage */
- goto abort_transaction;
+
+ pathsize = strlen(dev->nodename) + QUEUE_NAME_LEN;
+ path = kmalloc(pathsize, GFP_KERNEL);
+ if (!path) {
+ err = -ENOMEM;
+ message = "ENOMEM while writing ring references";
+ goto abort_transaction;
+ }
+
+ for (i = 0; i < info->nr_rings; i++) {
+ memset(path, 0, pathsize);
+ snprintf(path, pathsize, "%s/queue-%u", dev->nodename, i);
+ err = write_per_ring_nodes(xbt, &info->rinfo[i], path);
+ if (err) {
+ kfree(path);
+ goto destroy_blkring;
+ }
+ }
+ kfree(path);
}
+
err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
XEN_IO_PROTO_ABI_NATIVE);
if (err) {
@@ -1449,7 +1505,7 @@ again:
for (i = 0; i < info->nr_rings; i++) {
int j;
- rinfo = &info->rinfo[i];
+ struct blkfront_ring_info *rinfo = &info->rinfo[i];
for (j = 0; j < BLK_RING_SIZE(info); j++)
rinfo->shadow[j].req.u.rw.id = j + 1;
@@ -1480,6 +1536,7 @@ static int blkfront_probe(struct xenbus_device *dev,
{
int err, vdevice, r_index;
struct blkfront_info *info;
+ unsigned int backend_max_queues = 0;
/* FIXME: Use dynamic device id if this is not set. */
err = xenbus_scanf(XBT_NIL, dev->nodename,
@@ -1529,7 +1586,25 @@ static int blkfront_probe(struct xenbus_device *dev,
return -ENOMEM;
}
- info->nr_rings = 1;
+ mutex_init(&info->mutex);
+ spin_lock_init(&info->dev_lock);
+ info->xbdev = dev;
+ info->vdevice = vdevice;
+ INIT_LIST_HEAD(&info->grants);
+ info->connected = BLKIF_STATE_DISCONNECTED;
+
+ /* Check if backend supports multiple queues */
+ err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+ "multi-queue-max-queues", "%u", &backend_max_queues);
+ if (err < 0)
+ backend_max_queues = 1;
+
+ info->nr_rings = min(backend_max_queues, xen_blkif_max_queues);
+ if (info->nr_rings <= 0)
+ info->nr_rings = 1;
+ pr_debug("Number of queues to be used:%u, though backend supports max-queues:%u\n",
+ info->nr_rings, backend_max_queues);
+
info->rinfo = kzalloc(sizeof(struct blkfront_ring_info) * info->nr_rings, GFP_KERNEL);
if (!info->rinfo) {
xenbus_dev_fatal(dev, -ENOMEM, "allocating ring_info structure");
@@ -1547,14 +1622,6 @@ static int blkfront_probe(struct xenbus_device *dev,
spin_lock_init(&rinfo->ring_lock);
}
- mutex_init(&info->mutex);
- spin_lock_init(&info->dev_lock);
- info->xbdev = dev;
- info->vdevice = vdevice;
- INIT_LIST_HEAD(&info->grants);
- info->persistent_gnts_c = 0;
- info->connected = BLKIF_STATE_DISCONNECTED;
-
/* Front end dir is a number, which is used as the id. */
info->handle = simple_strtoul(strrchr(dev->nodename, '/')+1, NULL, 0);
dev_set_drvdata(&dev->dev, info);
@@ -2216,6 +2283,7 @@ static struct xenbus_driver blkfront_driver = {
static int __init xlblk_init(void)
{
int ret;
+ int nr_cpus = num_online_cpus();
if (!xen_domain())
return -ENODEV;
@@ -2226,6 +2294,12 @@ static int __init xlblk_init(void)
xen_blkif_max_ring_order = 0;
}
+ if (xen_blkif_max_queues > nr_cpus) {
+ pr_info("Invalid max_queues (%d), will use default max: %d.\n",
+ xen_blkif_max_queues, nr_cpus);
+ xen_blkif_max_queues = nr_cpus;
+ }
+
if (!xen_has_pv_disk_devices())
return -ENODEV;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 06/10] xen/blkback: separate ring information out of struct xen_blkif
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (4 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 05/10] xen/blkfront: negotiate number of queues/rings to be used with backend Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-02 4:21 ` [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings Bob Liu
` (4 subsequent siblings)
10 siblings, 0 replies; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Split per ring information to an new structure "xen_blkif_ring", so that one vbd
device can associate with one or more rings/hardware queues.
Introduce 'pers_gnts_lock' to protect the pool of persistent grants since we
may have multi backend threads.
This patch is a preparation for supporting multi hardware queues/rings.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkback/blkback.c | 233 ++++++++++++++++++++----------------
drivers/block/xen-blkback/common.h | 64 ++++++----
drivers/block/xen-blkback/xenbus.c | 107 ++++++++++-------
3 files changed, 234 insertions(+), 170 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 6a685ae..eaf7ec0 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -173,11 +173,11 @@ static inline void shrink_free_pagepool(struct xen_blkif *blkif, int num)
#define vaddr(page) ((unsigned long)pfn_to_kaddr(page_to_pfn(page)))
-static int do_block_io_op(struct xen_blkif *blkif);
-static int dispatch_rw_block_io(struct xen_blkif *blkif,
+static int do_block_io_op(struct xen_blkif_ring *ring);
+static int dispatch_rw_block_io(struct xen_blkif_ring *ring,
struct blkif_request *req,
struct pending_req *pending_req);
-static void make_response(struct xen_blkif *blkif, u64 id,
+static void make_response(struct xen_blkif_ring *ring, u64 id,
unsigned short op, int st);
#define foreach_grant_safe(pos, n, rbtree, node) \
@@ -189,14 +189,8 @@ static void make_response(struct xen_blkif *blkif, u64 id,
/*
- * We don't need locking around the persistent grant helpers
- * because blkback uses a single-thread for each backed, so we
- * can be sure that this functions will never be called recursively.
- *
- * The only exception to that is put_persistent_grant, that can be called
- * from interrupt context (by xen_blkbk_unmap), so we have to use atomic
- * bit operations to modify the flags of a persistent grant and to count
- * the number of used grants.
+ * pers_gnts_lock must be used around all the persistent grant helpers
+ * because blkback may use multi-thread/queue for each backend.
*/
static int add_persistent_gnt(struct xen_blkif *blkif,
struct persistent_gnt *persistent_gnt)
@@ -322,11 +316,13 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
int segs_to_unmap = 0;
struct xen_blkif *blkif = container_of(work, typeof(*blkif), persistent_purge_work);
struct gntab_unmap_queue_data unmap_data;
+ unsigned long flags;
unmap_data.pages = pages;
unmap_data.unmap_ops = unmap;
unmap_data.kunmap_ops = NULL;
+ spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
while(!list_empty(&blkif->persistent_purge_list)) {
persistent_gnt = list_first_entry(&blkif->persistent_purge_list,
struct persistent_gnt,
@@ -348,6 +344,7 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
}
kfree(persistent_gnt);
}
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
if (segs_to_unmap > 0) {
unmap_data.count = segs_to_unmap;
BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
@@ -362,16 +359,18 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)
unsigned int num_clean, total;
bool scan_used = false, clean_used = false;
struct rb_root *root;
+ unsigned long flags;
+ spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
if (blkif->persistent_gnt_c < xen_blkif_max_pgrants ||
(blkif->persistent_gnt_c == xen_blkif_max_pgrants &&
!blkif->vbd.overflow_max_grants)) {
- return;
+ goto out;
}
if (work_busy(&blkif->persistent_purge_work)) {
pr_alert_ratelimited("Scheduled work from previous purge is still busy, cannot purge list\n");
- return;
+ goto out;
}
num_clean = (xen_blkif_max_pgrants / 100) * LRU_PERCENT_CLEAN;
@@ -379,7 +378,7 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)
num_clean = min(blkif->persistent_gnt_c, num_clean);
if ((num_clean == 0) ||
(num_clean > (blkif->persistent_gnt_c - atomic_read(&blkif->persistent_gnt_in_use))))
- return;
+ goto out;
/*
* At this point, we can assure that there will be no calls
@@ -436,29 +435,35 @@ finished:
}
blkif->persistent_gnt_c -= (total - num_clean);
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
blkif->vbd.overflow_max_grants = 0;
/* We can defer this work */
schedule_work(&blkif->persistent_purge_work);
pr_debug("Purged %u/%u\n", (total - num_clean), total);
return;
+
+out:
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
+
+ return;
}
/*
* Retrieve from the 'pending_reqs' a free pending_req structure to be used.
*/
-static struct pending_req *alloc_req(struct xen_blkif *blkif)
+static struct pending_req *alloc_req(struct xen_blkif_ring *ring)
{
struct pending_req *req = NULL;
unsigned long flags;
- spin_lock_irqsave(&blkif->pending_free_lock, flags);
- if (!list_empty(&blkif->pending_free)) {
- req = list_entry(blkif->pending_free.next, struct pending_req,
+ spin_lock_irqsave(&ring->pending_free_lock, flags);
+ if (!list_empty(&ring->pending_free)) {
+ req = list_entry(ring->pending_free.next, struct pending_req,
free_list);
list_del(&req->free_list);
}
- spin_unlock_irqrestore(&blkif->pending_free_lock, flags);
+ spin_unlock_irqrestore(&ring->pending_free_lock, flags);
return req;
}
@@ -466,17 +471,17 @@ static struct pending_req *alloc_req(struct xen_blkif *blkif)
* Return the 'pending_req' structure back to the freepool. We also
* wake up the thread if it was waiting for a free page.
*/
-static void free_req(struct xen_blkif *blkif, struct pending_req *req)
+static void free_req(struct xen_blkif_ring *ring, struct pending_req *req)
{
unsigned long flags;
int was_empty;
- spin_lock_irqsave(&blkif->pending_free_lock, flags);
- was_empty = list_empty(&blkif->pending_free);
- list_add(&req->free_list, &blkif->pending_free);
- spin_unlock_irqrestore(&blkif->pending_free_lock, flags);
+ spin_lock_irqsave(&ring->pending_free_lock, flags);
+ was_empty = list_empty(&ring->pending_free);
+ list_add(&req->free_list, &ring->pending_free);
+ spin_unlock_irqrestore(&ring->pending_free_lock, flags);
if (was_empty)
- wake_up(&blkif->pending_free_wq);
+ wake_up(&ring->pending_free_wq);
}
/*
@@ -556,10 +561,10 @@ abort:
/*
* Notification from the guest OS.
*/
-static void blkif_notify_work(struct xen_blkif *blkif)
+static void blkif_notify_work(struct xen_blkif_ring *ring)
{
- blkif->waiting_reqs = 1;
- wake_up(&blkif->wq);
+ ring->waiting_reqs = 1;
+ wake_up(&ring->wq);
}
irqreturn_t xen_blkif_be_int(int irq, void *dev_id)
@@ -590,7 +595,8 @@ static void print_stats(struct xen_blkif *blkif)
int xen_blkif_schedule(void *arg)
{
- struct xen_blkif *blkif = arg;
+ struct xen_blkif_ring *ring = arg;
+ struct xen_blkif *blkif = ring->blkif;
struct xen_vbd *vbd = &blkif->vbd;
unsigned long timeout;
int ret;
@@ -606,27 +612,27 @@ int xen_blkif_schedule(void *arg)
timeout = msecs_to_jiffies(LRU_INTERVAL);
timeout = wait_event_interruptible_timeout(
- blkif->wq,
- blkif->waiting_reqs || kthread_should_stop(),
+ ring->wq,
+ ring->waiting_reqs || kthread_should_stop(),
timeout);
if (timeout == 0)
goto purge_gnt_list;
timeout = wait_event_interruptible_timeout(
- blkif->pending_free_wq,
- !list_empty(&blkif->pending_free) ||
+ ring->pending_free_wq,
+ !list_empty(&ring->pending_free) ||
kthread_should_stop(),
timeout);
if (timeout == 0)
goto purge_gnt_list;
- blkif->waiting_reqs = 0;
+ ring->waiting_reqs = 0;
smp_mb(); /* clear flag *before* checking for work */
- ret = do_block_io_op(blkif);
+ ret = do_block_io_op(ring);
if (ret > 0)
- blkif->waiting_reqs = 1;
+ ring->waiting_reqs = 1;
if (ret == -EACCES)
- wait_event_interruptible(blkif->shutdown_wq,
+ wait_event_interruptible(ring->shutdown_wq,
kthread_should_stop());
purge_gnt_list:
@@ -649,7 +655,7 @@ purge_gnt_list:
if (log_stats)
print_stats(blkif);
- blkif->xenblkd = NULL;
+ ring->xenblkd = NULL;
xen_blkif_put(blkif);
return 0;
@@ -658,32 +664,40 @@ purge_gnt_list:
/*
* Remove persistent grants and empty the pool of free pages
*/
-void xen_blkbk_free_caches(struct xen_blkif *blkif)
+void xen_blkbk_free_caches(struct xen_blkif_ring *ring)
{
+ struct xen_blkif *blkif = ring->blkif;
+ unsigned long flags;
+
/* Free all persistent grant pages */
+ spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
if (!RB_EMPTY_ROOT(&blkif->persistent_gnts))
free_persistent_gnts(blkif, &blkif->persistent_gnts,
blkif->persistent_gnt_c);
BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
blkif->persistent_gnt_c = 0;
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
/* Since we are shutting down remove all pages from the buffer */
shrink_free_pagepool(blkif, 0 /* All */);
}
static unsigned int xen_blkbk_unmap_prepare(
- struct xen_blkif *blkif,
+ struct xen_blkif_ring *ring,
struct grant_page **pages,
unsigned int num,
struct gnttab_unmap_grant_ref *unmap_ops,
struct page **unmap_pages)
{
unsigned int i, invcount = 0;
+ unsigned long flags;
for (i = 0; i < num; i++) {
if (pages[i]->persistent_gnt != NULL) {
- put_persistent_gnt(blkif, pages[i]->persistent_gnt);
+ spin_lock_irqsave(&ring->blkif->pers_gnts_lock, flags);
+ put_persistent_gnt(ring->blkif, pages[i]->persistent_gnt);
+ spin_unlock_irqrestore(&ring->blkif->pers_gnts_lock, flags);
continue;
}
if (pages[i]->handle == BLKBACK_INVALID_HANDLE)
@@ -700,17 +714,18 @@ static unsigned int xen_blkbk_unmap_prepare(
static void xen_blkbk_unmap_and_respond_callback(int result, struct gntab_unmap_queue_data *data)
{
- struct pending_req* pending_req = (struct pending_req*) (data->data);
- struct xen_blkif *blkif = pending_req->blkif;
+ struct pending_req *pending_req = (struct pending_req *)(data->data);
+ struct xen_blkif_ring *ring = pending_req->ring;
+ struct xen_blkif *blkif = ring->blkif;
/* BUG_ON used to reproduce existing behaviour,
but is this the best way to deal with this? */
BUG_ON(result);
put_free_pages(blkif, data->pages, data->count);
- make_response(blkif, pending_req->id,
+ make_response(ring, pending_req->id,
pending_req->operation, pending_req->status);
- free_req(blkif, pending_req);
+ free_req(ring, pending_req);
/*
* Make sure the request is freed before releasing blkif,
* or there could be a race between free_req and the
@@ -723,7 +738,7 @@ static void xen_blkbk_unmap_and_respond_callback(int result, struct gntab_unmap_
* pending_free_wq if there's a drain going on, but it has
* to be taken into account if the current model is changed.
*/
- if (atomic_dec_and_test(&blkif->inflight) && atomic_read(&blkif->drain)) {
+ if (atomic_dec_and_test(&ring->inflight) && atomic_read(&blkif->drain)) {
complete(&blkif->drain_complete);
}
xen_blkif_put(blkif);
@@ -732,11 +747,11 @@ static void xen_blkbk_unmap_and_respond_callback(int result, struct gntab_unmap_
static void xen_blkbk_unmap_and_respond(struct pending_req *req)
{
struct gntab_unmap_queue_data* work = &req->gnttab_unmap_data;
- struct xen_blkif *blkif = req->blkif;
+ struct xen_blkif_ring *ring = req->ring;
struct grant_page **pages = req->segments;
unsigned int invcount;
- invcount = xen_blkbk_unmap_prepare(blkif, pages, req->nr_segs,
+ invcount = xen_blkbk_unmap_prepare(ring, pages, req->nr_segs,
req->unmap, req->unmap_pages);
work->data = req;
@@ -757,7 +772,7 @@ static void xen_blkbk_unmap_and_respond(struct pending_req *req)
* of hypercalls, but since this is only used in error paths there's
* no real need.
*/
-static void xen_blkbk_unmap(struct xen_blkif *blkif,
+static void xen_blkbk_unmap(struct xen_blkif_ring *ring,
struct grant_page *pages[],
int num)
{
@@ -768,20 +783,20 @@ static void xen_blkbk_unmap(struct xen_blkif *blkif,
while (num) {
unsigned int batch = min(num, BLKIF_MAX_SEGMENTS_PER_REQUEST);
-
- invcount = xen_blkbk_unmap_prepare(blkif, pages, batch,
+
+ invcount = xen_blkbk_unmap_prepare(ring, pages, batch,
unmap, unmap_pages);
if (invcount) {
ret = gnttab_unmap_refs(unmap, NULL, unmap_pages, invcount);
BUG_ON(ret);
- put_free_pages(blkif, unmap_pages, invcount);
+ put_free_pages(ring->blkif, unmap_pages, invcount);
}
pages += batch;
num -= batch;
}
}
-static int xen_blkbk_map(struct xen_blkif *blkif,
+static int xen_blkbk_map(struct xen_blkif_ring *ring,
struct grant_page *pages[],
int num, bool ro)
{
@@ -794,6 +809,8 @@ static int xen_blkbk_map(struct xen_blkif *blkif,
int ret = 0;
int last_map = 0, map_until = 0;
int use_persistent_gnts;
+ struct xen_blkif *blkif = ring->blkif;
+ unsigned long irq_flags;
use_persistent_gnts = (blkif->vbd.feature_gnt_persistent);
@@ -806,10 +823,13 @@ again:
for (i = map_until; i < num; i++) {
uint32_t flags;
- if (use_persistent_gnts)
+ if (use_persistent_gnts) {
+ spin_lock_irqsave(&blkif->pers_gnts_lock, irq_flags);
persistent_gnt = get_persistent_gnt(
blkif,
pages[i]->gref);
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
+ }
if (persistent_gnt) {
/*
@@ -880,8 +900,10 @@ again:
persistent_gnt->gnt = map[new_map_idx].ref;
persistent_gnt->handle = map[new_map_idx].handle;
persistent_gnt->page = pages[seg_idx]->page;
+ spin_lock_irqsave(&blkif->pers_gnts_lock, irq_flags);
if (add_persistent_gnt(blkif,
persistent_gnt)) {
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
kfree(persistent_gnt);
persistent_gnt = NULL;
goto next;
@@ -890,6 +912,7 @@ again:
pr_debug("grant %u added to the tree of persistent grants, using %u/%u\n",
persistent_gnt->gnt, blkif->persistent_gnt_c,
xen_blkif_max_pgrants);
+ spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
goto next;
}
if (use_persistent_gnts && !blkif->vbd.overflow_max_grants) {
@@ -921,7 +944,7 @@ static int xen_blkbk_map_seg(struct pending_req *pending_req)
{
int rc;
- rc = xen_blkbk_map(pending_req->blkif, pending_req->segments,
+ rc = xen_blkbk_map(pending_req->ring, pending_req->segments,
pending_req->nr_segs,
(pending_req->operation != BLKIF_OP_READ));
@@ -934,7 +957,7 @@ static int xen_blkbk_parse_indirect(struct blkif_request *req,
struct phys_req *preq)
{
struct grant_page **pages = pending_req->indirect_pages;
- struct xen_blkif *blkif = pending_req->blkif;
+ struct xen_blkif_ring *ring = pending_req->ring;
int indirect_grefs, rc, n, nseg, i;
struct blkif_request_segment *segments = NULL;
@@ -945,7 +968,7 @@ static int xen_blkbk_parse_indirect(struct blkif_request *req,
for (i = 0; i < indirect_grefs; i++)
pages[i]->gref = req->u.indirect.indirect_grefs[i];
- rc = xen_blkbk_map(blkif, pages, indirect_grefs, true);
+ rc = xen_blkbk_map(ring, pages, indirect_grefs, true);
if (rc)
goto unmap;
@@ -972,15 +995,16 @@ static int xen_blkbk_parse_indirect(struct blkif_request *req,
unmap:
if (segments)
kunmap_atomic(segments);
- xen_blkbk_unmap(blkif, pages, indirect_grefs);
+ xen_blkbk_unmap(ring, pages, indirect_grefs);
return rc;
}
-static int dispatch_discard_io(struct xen_blkif *blkif,
+static int dispatch_discard_io(struct xen_blkif_ring *ring,
struct blkif_request *req)
{
int err = 0;
int status = BLKIF_RSP_OKAY;
+ struct xen_blkif *blkif = ring->blkif;
struct block_device *bdev = blkif->vbd.bdev;
unsigned long secure;
struct phys_req preq;
@@ -997,7 +1021,7 @@ static int dispatch_discard_io(struct xen_blkif *blkif,
preq.sector_number + preq.nr_sects, blkif->vbd.pdevice);
goto fail_response;
}
- blkif->st_ds_req++;
+ ring->st_ds_req++;
secure = (blkif->vbd.discard_secure &&
(req->u.discard.flag & BLKIF_DISCARD_SECURE)) ?
@@ -1013,26 +1037,28 @@ fail_response:
} else if (err)
status = BLKIF_RSP_ERROR;
- make_response(blkif, req->u.discard.id, req->operation, status);
+ make_response(ring, req->u.discard.id, req->operation, status);
xen_blkif_put(blkif);
return err;
}
-static int dispatch_other_io(struct xen_blkif *blkif,
+static int dispatch_other_io(struct xen_blkif_ring *ring,
struct blkif_request *req,
struct pending_req *pending_req)
{
- free_req(blkif, pending_req);
- make_response(blkif, req->u.other.id, req->operation,
+ free_req(ring, pending_req);
+ make_response(ring, req->u.other.id, req->operation,
BLKIF_RSP_EOPNOTSUPP);
return -EIO;
}
-static void xen_blk_drain_io(struct xen_blkif *blkif)
+static void xen_blk_drain_io(struct xen_blkif_ring *ring)
{
+ struct xen_blkif *blkif = ring->blkif;
+
atomic_set(&blkif->drain, 1);
do {
- if (atomic_read(&blkif->inflight) == 0)
+ if (atomic_read(&ring->inflight) == 0)
break;
wait_for_completion_interruptible_timeout(
&blkif->drain_complete, HZ);
@@ -1053,12 +1079,12 @@ static void __end_block_io_op(struct pending_req *pending_req, int error)
if ((pending_req->operation == BLKIF_OP_FLUSH_DISKCACHE) &&
(error == -EOPNOTSUPP)) {
pr_debug("flush diskcache op failed, not supported\n");
- xen_blkbk_flush_diskcache(XBT_NIL, pending_req->blkif->be, 0);
+ xen_blkbk_flush_diskcache(XBT_NIL, pending_req->ring->blkif->be, 0);
pending_req->status = BLKIF_RSP_EOPNOTSUPP;
} else if ((pending_req->operation == BLKIF_OP_WRITE_BARRIER) &&
(error == -EOPNOTSUPP)) {
pr_debug("write barrier op failed, not supported\n");
- xen_blkbk_barrier(XBT_NIL, pending_req->blkif->be, 0);
+ xen_blkbk_barrier(XBT_NIL, pending_req->ring->blkif->be, 0);
pending_req->status = BLKIF_RSP_EOPNOTSUPP;
} else if (error) {
pr_debug("Buffer not up-to-date at end of operation,"
@@ -1092,9 +1118,9 @@ static void end_block_io_op(struct bio *bio)
* and transmute it to the block API to hand it over to the proper block disk.
*/
static int
-__do_block_io_op(struct xen_blkif *blkif)
+__do_block_io_op(struct xen_blkif_ring *ring)
{
- union blkif_back_rings *blk_rings = &blkif->blk_rings;
+ union blkif_back_rings *blk_rings = &ring->blk_rings;
struct blkif_request req;
struct pending_req *pending_req;
RING_IDX rc, rp;
@@ -1107,7 +1133,7 @@ __do_block_io_op(struct xen_blkif *blkif)
if (RING_REQUEST_PROD_OVERFLOW(&blk_rings->common, rp)) {
rc = blk_rings->common.rsp_prod_pvt;
pr_warn("Frontend provided bogus ring requests (%d - %d = %d). Halting ring processing on dev=%04x\n",
- rp, rc, rp - rc, blkif->vbd.pdevice);
+ rp, rc, rp - rc, ring->blkif->vbd.pdevice);
return -EACCES;
}
while (rc != rp) {
@@ -1120,14 +1146,14 @@ __do_block_io_op(struct xen_blkif *blkif)
break;
}
- pending_req = alloc_req(blkif);
+ pending_req = alloc_req(ring);
if (NULL == pending_req) {
- blkif->st_oo_req++;
+ ring->st_oo_req++;
more_to_do = 1;
break;
}
- switch (blkif->blk_protocol) {
+ switch (ring->blkif->blk_protocol) {
case BLKIF_PROTOCOL_NATIVE:
memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req));
break;
@@ -1151,16 +1177,16 @@ __do_block_io_op(struct xen_blkif *blkif)
case BLKIF_OP_WRITE_BARRIER:
case BLKIF_OP_FLUSH_DISKCACHE:
case BLKIF_OP_INDIRECT:
- if (dispatch_rw_block_io(blkif, &req, pending_req))
+ if (dispatch_rw_block_io(ring, &req, pending_req))
goto done;
break;
case BLKIF_OP_DISCARD:
- free_req(blkif, pending_req);
- if (dispatch_discard_io(blkif, &req))
+ free_req(ring, pending_req);
+ if (dispatch_discard_io(ring, &req))
goto done;
break;
default:
- if (dispatch_other_io(blkif, &req, pending_req))
+ if (dispatch_other_io(ring, &req, pending_req))
goto done;
break;
}
@@ -1173,13 +1199,13 @@ done:
}
static int
-do_block_io_op(struct xen_blkif *blkif)
+do_block_io_op(struct xen_blkif_ring *ring)
{
- union blkif_back_rings *blk_rings = &blkif->blk_rings;
+ union blkif_back_rings *blk_rings = &ring->blk_rings;
int more_to_do;
do {
- more_to_do = __do_block_io_op(blkif);
+ more_to_do = __do_block_io_op(ring);
if (more_to_do)
break;
@@ -1192,7 +1218,7 @@ do_block_io_op(struct xen_blkif *blkif)
* Transmutation of the 'struct blkif_request' to a proper 'struct bio'
* and call the 'submit_bio' to pass it to the underlying storage.
*/
-static int dispatch_rw_block_io(struct xen_blkif *blkif,
+static int dispatch_rw_block_io(struct xen_blkif_ring *ring,
struct blkif_request *req,
struct pending_req *pending_req)
{
@@ -1219,17 +1245,17 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
switch (req_operation) {
case BLKIF_OP_READ:
- blkif->st_rd_req++;
+ ring->st_rd_req++;
operation = READ;
break;
case BLKIF_OP_WRITE:
- blkif->st_wr_req++;
+ ring->st_wr_req++;
operation = WRITE_ODIRECT;
break;
case BLKIF_OP_WRITE_BARRIER:
drain = true;
case BLKIF_OP_FLUSH_DISKCACHE:
- blkif->st_f_req++;
+ ring->st_f_req++;
operation = WRITE_FLUSH;
break;
default:
@@ -1254,7 +1280,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
preq.nr_sects = 0;
- pending_req->blkif = blkif;
+ pending_req->ring = ring;
pending_req->id = req->u.rw.id;
pending_req->operation = req_operation;
pending_req->status = BLKIF_RSP_OKAY;
@@ -1281,12 +1307,12 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
goto fail_response;
}
- if (xen_vbd_translate(&preq, blkif, operation) != 0) {
+ if (xen_vbd_translate(&preq, ring->blkif, operation) != 0) {
pr_debug("access denied: %s of [%llu,%llu] on dev=%04x\n",
operation == READ ? "read" : "write",
preq.sector_number,
preq.sector_number + preq.nr_sects,
- blkif->vbd.pdevice);
+ ring->blkif->vbd.pdevice);
goto fail_response;
}
@@ -1298,7 +1324,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
if (((int)preq.sector_number|(int)seg[i].nsec) &
((bdev_logical_block_size(preq.bdev) >> 9) - 1)) {
pr_debug("Misaligned I/O request from domain %d\n",
- blkif->domid);
+ ring->blkif->domid);
goto fail_response;
}
}
@@ -1307,7 +1333,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
* issue the WRITE_FLUSH.
*/
if (drain)
- xen_blk_drain_io(pending_req->blkif);
+ xen_blk_drain_io(pending_req->ring);
/*
* If we have failed at this point, we need to undo the M2P override,
@@ -1322,8 +1348,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
* This corresponding xen_blkif_put is done in __end_block_io_op, or
* below (in "!bio") if we are handling a BLKIF_OP_DISCARD.
*/
- xen_blkif_get(blkif);
- atomic_inc(&blkif->inflight);
+ xen_blkif_get(ring->blkif);
+ atomic_inc(&ring->inflight);
for (i = 0; i < nseg; i++) {
while ((bio == NULL) ||
@@ -1371,19 +1397,19 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
blk_finish_plug(&plug);
if (operation == READ)
- blkif->st_rd_sect += preq.nr_sects;
+ ring->st_rd_sect += preq.nr_sects;
else if (operation & WRITE)
- blkif->st_wr_sect += preq.nr_sects;
+ ring->st_wr_sect += preq.nr_sects;
return 0;
fail_flush:
- xen_blkbk_unmap(blkif, pending_req->segments,
+ xen_blkbk_unmap(ring, pending_req->segments,
pending_req->nr_segs);
fail_response:
/* Haven't submitted any bio's yet. */
- make_response(blkif, req->u.rw.id, req_operation, BLKIF_RSP_ERROR);
- free_req(blkif, pending_req);
+ make_response(ring, req->u.rw.id, req_operation, BLKIF_RSP_ERROR);
+ free_req(ring, pending_req);
msleep(1); /* back off a bit */
return -EIO;
@@ -1401,21 +1427,22 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
/*
* Put a response on the ring on how the operation fared.
*/
-static void make_response(struct xen_blkif *blkif, u64 id,
+static void make_response(struct xen_blkif_ring *ring, u64 id,
unsigned short op, int st)
{
struct blkif_response resp;
unsigned long flags;
- union blkif_back_rings *blk_rings = &blkif->blk_rings;
+ union blkif_back_rings *blk_rings;
int notify;
resp.id = id;
resp.operation = op;
resp.status = st;
- spin_lock_irqsave(&blkif->blk_ring_lock, flags);
+ spin_lock_irqsave(&ring->blk_ring_lock, flags);
+ blk_rings = &ring->blk_rings;
/* Place on the response ring for the relevant domain. */
- switch (blkif->blk_protocol) {
+ switch (ring->blkif->blk_protocol) {
case BLKIF_PROTOCOL_NATIVE:
memcpy(RING_GET_RESPONSE(&blk_rings->native, blk_rings->native.rsp_prod_pvt),
&resp, sizeof(resp));
@@ -1433,9 +1460,9 @@ static void make_response(struct xen_blkif *blkif, u64 id,
}
blk_rings->common.rsp_prod_pvt++;
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify);
- spin_unlock_irqrestore(&blkif->blk_ring_lock, flags);
+ spin_unlock_irqrestore(&ring->blk_ring_lock, flags);
if (notify)
- notify_remote_via_irq(blkif->irq);
+ notify_remote_via_irq(ring->irq);
}
static int __init xen_blkif_init(void)
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 45a044a..f0dd69a 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -260,34 +260,60 @@ struct persistent_gnt {
struct list_head remove_node;
};
+/* Per-ring information */
+struct xen_blkif_ring {
+ /* Physical parameters of the comms window. */
+ unsigned int irq;
+ union blkif_back_rings blk_rings;
+ void *blk_ring;
+ /* Private fields. */
+ spinlock_t blk_ring_lock;
+
+ wait_queue_head_t wq;
+ atomic_t inflight;
+ /* One thread per blkif ring. */
+ struct task_struct *xenblkd;
+ unsigned int waiting_reqs;
+
+ /* List of all 'pending_req' available */
+ struct list_head pending_free;
+ /* And its spinlock. */
+ spinlock_t pending_free_lock;
+ wait_queue_head_t pending_free_wq;
+
+ /* statistics */
+ unsigned long st_print;
+ unsigned long long st_rd_req;
+ unsigned long long st_wr_req;
+ unsigned long long st_oo_req;
+ unsigned long long st_f_req;
+ unsigned long long st_ds_req;
+ unsigned long long st_rd_sect;
+ unsigned long long st_wr_sect;
+
+ struct work_struct free_work;
+ /* Thread shutdown wait queue. */
+ wait_queue_head_t shutdown_wq;
+ struct xen_blkif *blkif;
+};
+
struct xen_blkif {
/* Unique identifier for this interface. */
domid_t domid;
unsigned int handle;
- /* Physical parameters of the comms window. */
- unsigned int irq;
/* Comms information. */
enum blkif_protocol blk_protocol;
- union blkif_back_rings blk_rings;
- void *blk_ring;
/* The VBD attached to this interface. */
struct xen_vbd vbd;
/* Back pointer to the backend_info. */
struct backend_info *be;
- /* Private fields. */
- spinlock_t blk_ring_lock;
atomic_t refcnt;
-
- wait_queue_head_t wq;
/* for barrier (drain) requests */
struct completion drain_complete;
atomic_t drain;
- atomic_t inflight;
- /* One thread per one blkif. */
- struct task_struct *xenblkd;
- unsigned int waiting_reqs;
/* tree to store persistent grants */
+ spinlock_t pers_gnts_lock;
struct rb_root persistent_gnts;
unsigned int persistent_gnt_c;
atomic_t persistent_gnt_in_use;
@@ -302,12 +328,6 @@ struct xen_blkif {
int free_pages_num;
struct list_head free_pages;
- /* List of all 'pending_req' available */
- struct list_head pending_free;
- /* And its spinlock. */
- spinlock_t pending_free_lock;
- wait_queue_head_t pending_free_wq;
-
/* statistics */
unsigned long st_print;
unsigned long long st_rd_req;
@@ -319,9 +339,9 @@ struct xen_blkif {
unsigned long long st_wr_sect;
struct work_struct free_work;
- /* Thread shutdown wait queue. */
- wait_queue_head_t shutdown_wq;
unsigned int nr_ring_pages;
+ /* All rings for this device */
+ struct xen_blkif_ring ring;
};
struct seg_buf {
@@ -343,7 +363,7 @@ struct grant_page {
* response queued for it, with the saved 'id' passed back.
*/
struct pending_req {
- struct xen_blkif *blkif;
+ struct xen_blkif_ring *ring;
u64 id;
int nr_segs;
atomic_t pendcnt;
@@ -385,7 +405,7 @@ int xen_blkif_xenbus_init(void);
irqreturn_t xen_blkif_be_int(int irq, void *dev_id);
int xen_blkif_schedule(void *arg);
int xen_blkif_purge_persistent(void *arg);
-void xen_blkbk_free_caches(struct xen_blkif *blkif);
+void xen_blkbk_free_caches(struct xen_blkif_ring *ring);
int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
struct backend_info *be, int state);
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 7676575..7bdd5fd 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -88,7 +88,7 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
char name[BLKBACK_NAME_LEN];
/* Not ready to connect? */
- if (!blkif->irq || !blkif->vbd.bdev)
+ if (!blkif->ring.irq || !blkif->vbd.bdev)
return;
/* Already connected? */
@@ -113,10 +113,10 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
}
invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
- blkif->xenblkd = kthread_run(xen_blkif_schedule, blkif, "%s", name);
- if (IS_ERR(blkif->xenblkd)) {
- err = PTR_ERR(blkif->xenblkd);
- blkif->xenblkd = NULL;
+ blkif->ring.xenblkd = kthread_run(xen_blkif_schedule, &blkif->ring, "%s", name);
+ if (IS_ERR(blkif->ring.xenblkd)) {
+ err = PTR_ERR(blkif->ring.xenblkd);
+ blkif->ring.xenblkd = NULL;
xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
return;
}
@@ -125,6 +125,7 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
static struct xen_blkif *xen_blkif_alloc(domid_t domid)
{
struct xen_blkif *blkif;
+ struct xen_blkif_ring *ring;
BUILD_BUG_ON(MAX_INDIRECT_PAGES > BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
@@ -133,41 +134,45 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
return ERR_PTR(-ENOMEM);
blkif->domid = domid;
- spin_lock_init(&blkif->blk_ring_lock);
atomic_set(&blkif->refcnt, 1);
- init_waitqueue_head(&blkif->wq);
init_completion(&blkif->drain_complete);
atomic_set(&blkif->drain, 0);
- blkif->st_print = jiffies;
- blkif->persistent_gnts.rb_node = NULL;
+ INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
- INIT_LIST_HEAD(&blkif->persistent_purge_list);
blkif->free_pages_num = 0;
+ blkif->persistent_gnts.rb_node = NULL;
+ INIT_LIST_HEAD(&blkif->persistent_purge_list);
atomic_set(&blkif->persistent_gnt_in_use, 0);
- atomic_set(&blkif->inflight, 0);
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
- INIT_LIST_HEAD(&blkif->pending_free);
- INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
- spin_lock_init(&blkif->pending_free_lock);
- init_waitqueue_head(&blkif->pending_free_wq);
- init_waitqueue_head(&blkif->shutdown_wq);
+ ring = &blkif->ring;
+ ring->blkif = blkif;
+ spin_lock_init(&ring->blk_ring_lock);
+ init_waitqueue_head(&ring->wq);
+ ring->st_print = jiffies;
+ atomic_set(&ring->inflight, 0);
+
+ INIT_LIST_HEAD(&ring->pending_free);
+ spin_lock_init(&ring->pending_free_lock);
+ init_waitqueue_head(&ring->pending_free_wq);
+ init_waitqueue_head(&ring->shutdown_wq);
return blkif;
}
-static int xen_blkif_map(struct xen_blkif *blkif, grant_ref_t *gref,
+static int xen_blkif_map(struct xen_blkif_ring *ring, grant_ref_t *gref,
unsigned int nr_grefs, unsigned int evtchn)
{
int err;
+ struct xen_blkif *blkif = ring->blkif;
/* Already connected through? */
- if (blkif->irq)
+ if (ring->irq)
return 0;
err = xenbus_map_ring_valloc(blkif->be->dev, gref, nr_grefs,
- &blkif->blk_ring);
+ &ring->blk_ring);
if (err < 0)
return err;
@@ -175,22 +180,22 @@ static int xen_blkif_map(struct xen_blkif *blkif, grant_ref_t *gref,
case BLKIF_PROTOCOL_NATIVE:
{
struct blkif_sring *sring;
- sring = (struct blkif_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.native, sring, PAGE_SIZE * nr_grefs);
+ sring = (struct blkif_sring *)ring->blk_ring;
+ BACK_RING_INIT(&ring->blk_rings.native, sring, PAGE_SIZE * nr_grefs);
break;
}
case BLKIF_PROTOCOL_X86_32:
{
struct blkif_x86_32_sring *sring_x86_32;
- sring_x86_32 = (struct blkif_x86_32_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE * nr_grefs);
+ sring_x86_32 = (struct blkif_x86_32_sring *)ring->blk_ring;
+ BACK_RING_INIT(&ring->blk_rings.x86_32, sring_x86_32, PAGE_SIZE * nr_grefs);
break;
}
case BLKIF_PROTOCOL_X86_64:
{
struct blkif_x86_64_sring *sring_x86_64;
- sring_x86_64 = (struct blkif_x86_64_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE * nr_grefs);
+ sring_x86_64 = (struct blkif_x86_64_sring *)ring->blk_ring;
+ BACK_RING_INIT(&ring->blk_rings.x86_64, sring_x86_64, PAGE_SIZE * nr_grefs);
break;
}
default:
@@ -199,13 +204,13 @@ static int xen_blkif_map(struct xen_blkif *blkif, grant_ref_t *gref,
err = bind_interdomain_evtchn_to_irqhandler(blkif->domid, evtchn,
xen_blkif_be_int, 0,
- "blkif-backend", blkif);
+ "blkif-backend", ring);
if (err < 0) {
- xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
- blkif->blk_rings.common.sring = NULL;
+ xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
+ ring->blk_rings.common.sring = NULL;
return err;
}
- blkif->irq = err;
+ ring->irq = err;
return 0;
}
@@ -214,35 +219,36 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
{
struct pending_req *req, *n;
int i = 0, j;
+ struct xen_blkif_ring *ring = &blkif->ring;
- if (blkif->xenblkd) {
- kthread_stop(blkif->xenblkd);
- wake_up(&blkif->shutdown_wq);
- blkif->xenblkd = NULL;
+ if (ring->xenblkd) {
+ kthread_stop(ring->xenblkd);
+ wake_up(&ring->shutdown_wq);
+ ring->xenblkd = NULL;
}
/* The above kthread_stop() guarantees that at this point we
* don't have any discard_io or other_io requests. So, checking
* for inflight IO is enough.
*/
- if (atomic_read(&blkif->inflight) > 0)
+ if (atomic_read(&ring->inflight) > 0)
return -EBUSY;
- if (blkif->irq) {
- unbind_from_irqhandler(blkif->irq, blkif);
- blkif->irq = 0;
+ if (ring->irq) {
+ unbind_from_irqhandler(ring->irq, ring);
+ ring->irq = 0;
}
- if (blkif->blk_rings.common.sring) {
- xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
- blkif->blk_rings.common.sring = NULL;
+ if (ring->blk_rings.common.sring) {
+ xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
+ ring->blk_rings.common.sring = NULL;
}
/* Remove all persistent grants and the cache of ballooned pages. */
- xen_blkbk_free_caches(blkif);
+ xen_blkbk_free_caches(ring);
/* Check that there is no request in use */
- list_for_each_entry_safe(req, n, &blkif->pending_free, free_list) {
+ list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
list_del(&req->free_list);
for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
@@ -300,6 +306,16 @@ int __init xen_blkif_interface_init(void)
{ \
struct xenbus_device *dev = to_xenbus_device(_dev); \
struct backend_info *be = dev_get_drvdata(&dev->dev); \
+ struct xen_blkif *blkif = be->blkif; \
+ struct xen_blkif_ring *ring = &blkif->ring; \
+ \
+ blkif->st_oo_req = ring->st_oo_req; \
+ blkif->st_rd_req = ring->st_rd_req; \
+ blkif->st_wr_req = ring->st_wr_req; \
+ blkif->st_f_req = ring->st_f_req; \
+ blkif->st_ds_req = ring->st_ds_req; \
+ blkif->st_rd_sect = ring->st_rd_sect; \
+ blkif->st_wr_sect = ring->st_wr_sect; \
\
return sprintf(buf, format, ##args); \
} \
@@ -832,6 +848,7 @@ static int connect_ring(struct backend_info *be)
char protocol[64] = "";
struct pending_req *req, *n;
int err, i, j;
+ struct xen_blkif_ring *ring = &be->blkif->ring;
pr_debug("%s %s\n", __func__, dev->otherend);
@@ -920,7 +937,7 @@ static int connect_ring(struct backend_info *be)
req = kzalloc(sizeof(*req), GFP_KERNEL);
if (!req)
goto fail;
- list_add_tail(&req->free_list, &be->blkif->pending_free);
+ list_add_tail(&req->free_list, &ring->pending_free);
for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++) {
req->segments[j] = kzalloc(sizeof(*req->segments[0]), GFP_KERNEL);
if (!req->segments[j])
@@ -935,7 +952,7 @@ static int connect_ring(struct backend_info *be)
}
/* Map the shared frame, irq etc. */
- err = xen_blkif_map(be->blkif, ring_ref, nr_grefs, evtchn);
+ err = xen_blkif_map(ring, ring_ref, nr_grefs, evtchn);
if (err) {
xenbus_dev_fatal(dev, err, "mapping ring-ref port %u", evtchn);
return err;
@@ -944,7 +961,7 @@ static int connect_ring(struct backend_info *be)
return 0;
fail:
- list_for_each_entry_safe(req, n, &be->blkif->pending_free, free_list) {
+ list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
list_del(&req->free_list);
for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++) {
if (!req->segments[j])
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (5 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 06/10] xen/blkback: separate ring information out of struct xen_blkif Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-05 2:30 ` Konrad Rzeszutek Wilk
2015-11-02 4:21 ` [PATCH v4 08/10] xen/blkback: get the number of hardware queues/rings from blkfront Bob Liu
` (3 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Preparatory patch for multiple hardware queues (rings). The number of
rings is unconditionally set to 1, larger number will be enabled in next
patch so as to make every single patch small and readable.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkback/common.h | 3 +-
drivers/block/xen-blkback/xenbus.c | 292 +++++++++++++++++++++++--------------
2 files changed, 185 insertions(+), 110 deletions(-)
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index f0dd69a..4de1326 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -341,7 +341,8 @@ struct xen_blkif {
struct work_struct free_work;
unsigned int nr_ring_pages;
/* All rings for this device */
- struct xen_blkif_ring ring;
+ struct xen_blkif_ring *rings;
+ unsigned int nr_rings;
};
struct seg_buf {
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 7bdd5fd..ac4b458 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -84,11 +84,12 @@ static int blkback_name(struct xen_blkif *blkif, char *buf)
static void xen_update_blkif_status(struct xen_blkif *blkif)
{
- int err;
+ int err, i;
char name[BLKBACK_NAME_LEN];
+ struct xen_blkif_ring *ring;
/* Not ready to connect? */
- if (!blkif->ring.irq || !blkif->vbd.bdev)
+ if (!blkif->rings || !blkif->rings[0].irq || !blkif->vbd.bdev)
return;
/* Already connected? */
@@ -113,19 +114,57 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
}
invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
- blkif->ring.xenblkd = kthread_run(xen_blkif_schedule, &blkif->ring, "%s", name);
- if (IS_ERR(blkif->ring.xenblkd)) {
- err = PTR_ERR(blkif->ring.xenblkd);
- blkif->ring.xenblkd = NULL;
- xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
- return;
+ if (blkif->nr_rings == 1) {
+ blkif->rings[0].xenblkd = kthread_run(xen_blkif_schedule, &blkif->rings[0], "%s", name);
+ if (IS_ERR(blkif->rings[0].xenblkd)) {
+ err = PTR_ERR(blkif->rings[0].xenblkd);
+ blkif->rings[0].xenblkd = NULL;
+ xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
+ return;
+ }
+ } else {
+ for (i = 0; i < blkif->nr_rings; i++) {
+ ring = &blkif->rings[i];
+ ring->xenblkd = kthread_run(xen_blkif_schedule, ring, "%s-%d", name, i);
+ if (IS_ERR(ring->xenblkd)) {
+ err = PTR_ERR(ring->xenblkd);
+ ring->xenblkd = NULL;
+ xenbus_dev_error(blkif->be->dev, err,
+ "start %s-%d xenblkd", name, i);
+ return;
+ }
+ }
+ }
+}
+
+static int xen_blkif_alloc_rings(struct xen_blkif *blkif)
+{
+ int r;
+
+ blkif->rings = kzalloc(blkif->nr_rings * sizeof(struct xen_blkif_ring), GFP_KERNEL);
+ if (!blkif->rings)
+ return -ENOMEM;
+
+ for (r = 0; r < blkif->nr_rings; r++) {
+ struct xen_blkif_ring *ring = &blkif->rings[r];
+
+ spin_lock_init(&ring->blk_ring_lock);
+ init_waitqueue_head(&ring->wq);
+ INIT_LIST_HEAD(&ring->pending_free);
+
+ spin_lock_init(&ring->pending_free_lock);
+ init_waitqueue_head(&ring->pending_free_wq);
+ init_waitqueue_head(&ring->shutdown_wq);
+ ring->blkif = blkif;
+ xen_blkif_get(blkif);
}
+
+ return 0;
}
static struct xen_blkif *xen_blkif_alloc(domid_t domid)
{
struct xen_blkif *blkif;
- struct xen_blkif_ring *ring;
BUILD_BUG_ON(MAX_INDIRECT_PAGES > BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
@@ -136,27 +175,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
blkif->domid = domid;
atomic_set(&blkif->refcnt, 1);
init_completion(&blkif->drain_complete);
- atomic_set(&blkif->drain, 0);
INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
- blkif->free_pages_num = 0;
- blkif->persistent_gnts.rb_node = NULL;
INIT_LIST_HEAD(&blkif->persistent_purge_list);
- atomic_set(&blkif->persistent_gnt_in_use, 0);
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
- ring = &blkif->ring;
- ring->blkif = blkif;
- spin_lock_init(&ring->blk_ring_lock);
- init_waitqueue_head(&ring->wq);
- ring->st_print = jiffies;
- atomic_set(&ring->inflight, 0);
-
- INIT_LIST_HEAD(&ring->pending_free);
- spin_lock_init(&ring->pending_free_lock);
- init_waitqueue_head(&ring->pending_free_wq);
- init_waitqueue_head(&ring->shutdown_wq);
+ blkif->nr_rings = 1;
+ if (xen_blkif_alloc_rings(blkif)) {
+ kmem_cache_free(xen_blkif_cachep, blkif);
+ return ERR_PTR(-ENOMEM);
+ }
return blkif;
}
@@ -218,50 +247,54 @@ static int xen_blkif_map(struct xen_blkif_ring *ring, grant_ref_t *gref,
static int xen_blkif_disconnect(struct xen_blkif *blkif)
{
struct pending_req *req, *n;
- int i = 0, j;
- struct xen_blkif_ring *ring = &blkif->ring;
+ int j, r;
- if (ring->xenblkd) {
- kthread_stop(ring->xenblkd);
- wake_up(&ring->shutdown_wq);
- ring->xenblkd = NULL;
- }
+ for (r = 0; r < blkif->nr_rings; r++) {
+ struct xen_blkif_ring *ring = &blkif->rings[r];
+ int i = 0;
- /* The above kthread_stop() guarantees that at this point we
- * don't have any discard_io or other_io requests. So, checking
- * for inflight IO is enough.
- */
- if (atomic_read(&ring->inflight) > 0)
- return -EBUSY;
+ if (ring->xenblkd) {
+ kthread_stop(ring->xenblkd);
+ wake_up(&ring->shutdown_wq);
+ ring->xenblkd = NULL;
+ }
- if (ring->irq) {
- unbind_from_irqhandler(ring->irq, ring);
- ring->irq = 0;
- }
+ /* The above kthread_stop() guarantees that at this point we
+ * don't have any discard_io or other_io requests. So, checking
+ * for inflight IO is enough.
+ */
+ if (atomic_read(&ring->inflight) > 0)
+ return -EBUSY;
- if (ring->blk_rings.common.sring) {
- xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
- ring->blk_rings.common.sring = NULL;
- }
+ if (ring->irq) {
+ unbind_from_irqhandler(ring->irq, ring);
+ ring->irq = 0;
+ }
- /* Remove all persistent grants and the cache of ballooned pages. */
- xen_blkbk_free_caches(ring);
+ if (ring->blk_rings.common.sring) {
+ xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
+ ring->blk_rings.common.sring = NULL;
+ }
- /* Check that there is no request in use */
- list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
- list_del(&req->free_list);
+ /* Remove all persistent grants and the cache of ballooned pages. */
+ xen_blkbk_free_caches(ring);
- for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
- kfree(req->segments[j]);
+ /* Check that there is no request in use */
+ list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
+ list_del(&req->free_list);
- for (j = 0; j < MAX_INDIRECT_PAGES; j++)
- kfree(req->indirect_pages[j]);
+ for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
+ kfree(req->segments[j]);
- kfree(req);
- i++;
- }
+ for (j = 0; j < MAX_INDIRECT_PAGES; j++)
+ kfree(req->indirect_pages[j]);
- WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
+ kfree(req);
+ i++;
+ }
+
+ WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
+ }
blkif->nr_ring_pages = 0;
return 0;
@@ -281,6 +314,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
BUG_ON(!list_empty(&blkif->free_pages));
BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
+ kfree(blkif->rings);
kmem_cache_free(xen_blkif_cachep, blkif);
}
@@ -307,15 +341,19 @@ int __init xen_blkif_interface_init(void)
struct xenbus_device *dev = to_xenbus_device(_dev); \
struct backend_info *be = dev_get_drvdata(&dev->dev); \
struct xen_blkif *blkif = be->blkif; \
- struct xen_blkif_ring *ring = &blkif->ring; \
+ int i; \
+ \
+ for (i = 0; i < blkif->nr_rings; i++) { \
+ struct xen_blkif_ring *ring = &blkif->rings[i]; \
\
- blkif->st_oo_req = ring->st_oo_req; \
- blkif->st_rd_req = ring->st_rd_req; \
- blkif->st_wr_req = ring->st_wr_req; \
- blkif->st_f_req = ring->st_f_req; \
- blkif->st_ds_req = ring->st_ds_req; \
- blkif->st_rd_sect = ring->st_rd_sect; \
- blkif->st_wr_sect = ring->st_wr_sect; \
+ blkif->st_oo_req += ring->st_oo_req; \
+ blkif->st_rd_req += ring->st_rd_req; \
+ blkif->st_wr_req += ring->st_wr_req; \
+ blkif->st_f_req += ring->st_f_req; \
+ blkif->st_ds_req += ring->st_ds_req; \
+ blkif->st_rd_sect += ring->st_rd_sect; \
+ blkif->st_wr_sect += ring->st_wr_sect; \
+ } \
\
return sprintf(buf, format, ##args); \
} \
@@ -439,6 +477,7 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
static int xen_blkbk_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
+ int i;
pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
@@ -455,7 +494,8 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
if (be->blkif) {
xen_blkif_disconnect(be->blkif);
- xen_blkif_put(be->blkif);
+ for (i = 0; i < be->blkif->nr_rings; i++)
+ xen_blkif_put(be->blkif);
}
kfree(be->mode);
@@ -839,20 +879,16 @@ again:
}
-static int connect_ring(struct backend_info *be)
+static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
{
- struct xenbus_device *dev = be->dev;
+ unsigned int ring_page_order, nr_grefs, evtchn;
unsigned int ring_ref[XENBUS_MAX_RING_PAGES];
- unsigned int evtchn, nr_grefs, ring_page_order;
- unsigned int pers_grants;
- char protocol[64] = "";
struct pending_req *req, *n;
int err, i, j;
- struct xen_blkif_ring *ring = &be->blkif->ring;
-
- pr_debug("%s %s\n", __func__, dev->otherend);
+ struct xen_blkif *blkif = ring->blkif;
+ struct xenbus_device *dev = blkif->be->dev;
- err = xenbus_scanf(XBT_NIL, dev->otherend, "event-channel", "%u",
+ err = xenbus_scanf(XBT_NIL, dir, "event-channel", "%u",
&evtchn);
if (err != 1) {
err = -EINVAL;
@@ -860,12 +896,11 @@ static int connect_ring(struct backend_info *be)
dev->otherend);
return err;
}
- pr_info("event-channel %u\n", evtchn);
err = xenbus_scanf(XBT_NIL, dev->otherend, "ring-page-order", "%u",
&ring_page_order);
if (err != 1) {
- err = xenbus_scanf(XBT_NIL, dev->otherend, "ring-ref",
+ err = xenbus_scanf(XBT_NIL, dir, "ring-ref",
"%u", &ring_ref[0]);
if (err != 1) {
err = -EINVAL;
@@ -874,7 +909,7 @@ static int connect_ring(struct backend_info *be)
return err;
}
nr_grefs = 1;
- pr_info("%s:using single page: ring-ref %d\n", dev->otherend,
+ pr_info("%s:using single page: ring-ref %d\n", dir,
ring_ref[0]);
} else {
unsigned int i;
@@ -892,7 +927,7 @@ static int connect_ring(struct backend_info *be)
char ring_ref_name[RINGREF_NAME_LEN];
snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
- err = xenbus_scanf(XBT_NIL, dev->otherend, ring_ref_name,
+ err = xenbus_scanf(XBT_NIL, dir, ring_ref_name,
"%u", &ring_ref[i]);
if (err != 1) {
err = -EINVAL;
@@ -900,38 +935,12 @@ static int connect_ring(struct backend_info *be)
dev->otherend, ring_ref_name);
return err;
}
- pr_info("ring-ref%u: %u\n", i, ring_ref[i]);
+ pr_info("%s: ring-ref%u: %u\n", dir, i, ring_ref[i]);
}
}
+ blkif->nr_ring_pages = nr_grefs;
- be->blkif->blk_protocol = BLKIF_PROTOCOL_DEFAULT;
- err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
- "%63s", protocol, NULL);
- if (err)
- strcpy(protocol, "unspecified, assuming default");
- else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
- be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
- else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
- be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32;
- else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64))
- be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
- else {
- xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
- return -1;
- }
- err = xenbus_gather(XBT_NIL, dev->otherend,
- "feature-persistent", "%u",
- &pers_grants, NULL);
- if (err)
- pers_grants = 0;
-
- be->blkif->vbd.feature_gnt_persistent = pers_grants;
- be->blkif->vbd.overflow_max_grants = 0;
- be->blkif->nr_ring_pages = nr_grefs;
-
- pr_info("ring-pages:%d, event-channel %d, protocol %d (%s) %s\n",
- nr_grefs, evtchn, be->blkif->blk_protocol, protocol,
- pers_grants ? "persistent grants" : "");
+ pr_info("ring-pages:%d, event-channel %d.\n", nr_grefs, evtchn);
for (i = 0; i < nr_grefs * XEN_BLKIF_REQS_PER_PAGE; i++) {
req = kzalloc(sizeof(*req), GFP_KERNEL);
@@ -976,6 +985,71 @@ fail:
kfree(req);
}
return -ENOMEM;
+
+}
+
+static int connect_ring(struct backend_info *be)
+{
+ struct xenbus_device *dev = be->dev;
+ unsigned int pers_grants;
+ char protocol[64] = "";
+ int err, i;
+ char *xspath;
+ size_t xspathsize;
+ const size_t xenstore_path_ext_size = 11; /* sufficient for "/queue-NNN" */
+
+ pr_debug("%s %s\n", __func__, dev->otherend);
+
+ be->blkif->blk_protocol = BLKIF_PROTOCOL_DEFAULT;
+ err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
+ "%63s", protocol, NULL);
+ if (err)
+ strcpy(protocol, "unspecified, assuming default");
+ else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
+ be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
+ else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
+ be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32;
+ else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64))
+ be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
+ else {
+ xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
+ return -1;
+ }
+ err = xenbus_gather(XBT_NIL, dev->otherend,
+ "feature-persistent", "%u",
+ &pers_grants, NULL);
+ if (err)
+ pers_grants = 0;
+
+ be->blkif->vbd.feature_gnt_persistent = pers_grants;
+ be->blkif->vbd.overflow_max_grants = 0;
+
+ pr_info("nr_rings:%d protocol %d (%s) %s\n", be->blkif->nr_rings,
+ be->blkif->blk_protocol, protocol,
+ pers_grants ? "persistent grants" : "");
+
+ if (be->blkif->nr_rings == 1)
+ return read_per_ring_refs(&be->blkif->rings[0], dev->otherend);
+ else {
+ xspathsize = strlen(dev->otherend) + xenstore_path_ext_size;
+ xspath = kmalloc(xspathsize, GFP_KERNEL);
+ if (!xspath) {
+ xenbus_dev_fatal(dev, -ENOMEM, "reading ring references");
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < be->blkif->nr_rings; i++) {
+ memset(xspath, 0, xspathsize);
+ snprintf(xspath, xspathsize, "%s/queue-%u", dev->otherend, i);
+ err = read_per_ring_refs(&be->blkif->rings[i], xspath);
+ if (err) {
+ kfree(xspath);
+ return err;
+ }
+ }
+ kfree(xspath);
+ }
+ return 0;
}
static const struct xenbus_device_id xen_blkbk_ids[] = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings
2015-11-02 4:21 ` [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings Bob Liu
@ 2015-11-05 2:30 ` Konrad Rzeszutek Wilk
2015-11-05 3:02 ` Bob Liu
0 siblings, 1 reply; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 2:30 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On Mon, Nov 02, 2015 at 12:21:43PM +0800, Bob Liu wrote:
> Preparatory patch for multiple hardware queues (rings). The number of
> rings is unconditionally set to 1, larger number will be enabled in next
> patch so as to make every single patch small and readable.
Instead of saying 'next patch' - spell out the title of the patch.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> ---
> drivers/block/xen-blkback/common.h | 3 +-
> drivers/block/xen-blkback/xenbus.c | 292 +++++++++++++++++++++++--------------
> 2 files changed, 185 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
> index f0dd69a..4de1326 100644
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -341,7 +341,8 @@ struct xen_blkif {
> struct work_struct free_work;
> unsigned int nr_ring_pages;
> /* All rings for this device */
> - struct xen_blkif_ring ring;
> + struct xen_blkif_ring *rings;
> + unsigned int nr_rings;
> };
>
> struct seg_buf {
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 7bdd5fd..ac4b458 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -84,11 +84,12 @@ static int blkback_name(struct xen_blkif *blkif, char *buf)
>
> static void xen_update_blkif_status(struct xen_blkif *blkif)
> {
> - int err;
> + int err, i;
unsigned int.
> char name[BLKBACK_NAME_LEN];
> + struct xen_blkif_ring *ring;
>
> /* Not ready to connect? */
> - if (!blkif->ring.irq || !blkif->vbd.bdev)
> + if (!blkif->rings || !blkif->rings[0].irq || !blkif->vbd.bdev)
> return;
>
> /* Already connected? */
> @@ -113,19 +114,57 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
> }
> invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
>
> - blkif->ring.xenblkd = kthread_run(xen_blkif_schedule, &blkif->ring, "%s", name);
> - if (IS_ERR(blkif->ring.xenblkd)) {
> - err = PTR_ERR(blkif->ring.xenblkd);
> - blkif->ring.xenblkd = NULL;
> - xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
> - return;
> + if (blkif->nr_rings == 1) {
> + blkif->rings[0].xenblkd = kthread_run(xen_blkif_schedule, &blkif->rings[0], "%s", name);
> + if (IS_ERR(blkif->rings[0].xenblkd)) {
> + err = PTR_ERR(blkif->rings[0].xenblkd);
> + blkif->rings[0].xenblkd = NULL;
> + xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
> + return;
> + }
> + } else {
> + for (i = 0; i < blkif->nr_rings; i++) {
> + ring = &blkif->rings[i];
> + ring->xenblkd = kthread_run(xen_blkif_schedule, ring, "%s-%d", name, i);
> + if (IS_ERR(ring->xenblkd)) {
> + err = PTR_ERR(ring->xenblkd);
> + ring->xenblkd = NULL;
> + xenbus_dev_error(blkif->be->dev, err,
> + "start %s-%d xenblkd", name, i);
> + return;
> + }
> + }
Please collapse this together and just have one loop.
And just use the same name throughout ('%s-%d')
Also this does not take care of actually freeing the allocated
ring->xenblkd if one of them fails to start.
Hmm, looking at this function .. we seem to forget to change the
state if something fails.
As in, connect switches the state to Connected.. And if anything blows
up after the connect() we don't switch the state. We do report an error
in the XenBus, but the state is the same.
We should be using xenbus_dev_fatal I believe - so at least the state
changes to Closing (and then the frontend can switch itself to
Closing->Closed) - and we will call 'xen_blkif_disconnect' on Closed.
> + }
> +}
> +
> +static int xen_blkif_alloc_rings(struct xen_blkif *blkif)
> +{
> + int r;
unsigned int i;
> +
> + blkif->rings = kzalloc(blkif->nr_rings * sizeof(struct xen_blkif_ring), GFP_KERNEL);
> + if (!blkif->rings)
> + return -ENOMEM;
> +
> + for (r = 0; r < blkif->nr_rings; r++) {
> + struct xen_blkif_ring *ring = &blkif->rings[r];
> +
> + spin_lock_init(&ring->blk_ring_lock);
> + init_waitqueue_head(&ring->wq);
> + INIT_LIST_HEAD(&ring->pending_free);
> +
> + spin_lock_init(&ring->pending_free_lock);
> + init_waitqueue_head(&ring->pending_free_wq);
> + init_waitqueue_head(&ring->shutdown_wq);
> + ring->blkif = blkif;
> + xen_blkif_get(blkif);
> }
> +
> + return 0;
> }
>
> static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> {
> struct xen_blkif *blkif;
> - struct xen_blkif_ring *ring;
>
> BUILD_BUG_ON(MAX_INDIRECT_PAGES > BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
>
> @@ -136,27 +175,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> blkif->domid = domid;
> atomic_set(&blkif->refcnt, 1);
> init_completion(&blkif->drain_complete);
> - atomic_set(&blkif->drain, 0);
> INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
> spin_lock_init(&blkif->free_pages_lock);
> INIT_LIST_HEAD(&blkif->free_pages);
> - blkif->free_pages_num = 0;
> - blkif->persistent_gnts.rb_node = NULL;
> INIT_LIST_HEAD(&blkif->persistent_purge_list);
> - atomic_set(&blkif->persistent_gnt_in_use, 0);
> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>
> - ring = &blkif->ring;
> - ring->blkif = blkif;
> - spin_lock_init(&ring->blk_ring_lock);
> - init_waitqueue_head(&ring->wq);
> - ring->st_print = jiffies;
> - atomic_set(&ring->inflight, 0);
> -
> - INIT_LIST_HEAD(&ring->pending_free);
> - spin_lock_init(&ring->pending_free_lock);
> - init_waitqueue_head(&ring->pending_free_wq);
> - init_waitqueue_head(&ring->shutdown_wq);
> + blkif->nr_rings = 1;
> + if (xen_blkif_alloc_rings(blkif)) {
> + kmem_cache_free(xen_blkif_cachep, blkif);
> + return ERR_PTR(-ENOMEM);
> + }
>
> return blkif;
> }
> @@ -218,50 +247,54 @@ static int xen_blkif_map(struct xen_blkif_ring *ring, grant_ref_t *gref,
> static int xen_blkif_disconnect(struct xen_blkif *blkif)
> {
> struct pending_req *req, *n;
> - int i = 0, j;
> - struct xen_blkif_ring *ring = &blkif->ring;
> + int j, r;
>
unsigned int i;
> - if (ring->xenblkd) {
> - kthread_stop(ring->xenblkd);
> - wake_up(&ring->shutdown_wq);
> - ring->xenblkd = NULL;
> - }
> + for (r = 0; r < blkif->nr_rings; r++) {
> + struct xen_blkif_ring *ring = &blkif->rings[r];
> + int i = 0;
unsigned int
>
> - /* The above kthread_stop() guarantees that at this point we
> - * don't have any discard_io or other_io requests. So, checking
> - * for inflight IO is enough.
> - */
> - if (atomic_read(&ring->inflight) > 0)
> - return -EBUSY;
> + if (ring->xenblkd) {
> + kthread_stop(ring->xenblkd);
> + wake_up(&ring->shutdown_wq);
> + ring->xenblkd = NULL;
> + }
>
> - if (ring->irq) {
> - unbind_from_irqhandler(ring->irq, ring);
> - ring->irq = 0;
> - }
> + /* The above kthread_stop() guarantees that at this point we
> + * don't have any discard_io or other_io requests. So, checking
> + * for inflight IO is enough.
> + */
> + if (atomic_read(&ring->inflight) > 0)
> + return -EBUSY;
>
> - if (ring->blk_rings.common.sring) {
> - xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
> - ring->blk_rings.common.sring = NULL;
> - }
> + if (ring->irq) {
> + unbind_from_irqhandler(ring->irq, ring);
> + ring->irq = 0;
> + }
>
> - /* Remove all persistent grants and the cache of ballooned pages. */
> - xen_blkbk_free_caches(ring);
> + if (ring->blk_rings.common.sring) {
> + xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
> + ring->blk_rings.common.sring = NULL;
> + }
>
> - /* Check that there is no request in use */
> - list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
> - list_del(&req->free_list);
> + /* Remove all persistent grants and the cache of ballooned pages. */
> + xen_blkbk_free_caches(ring);
>
> - for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
> - kfree(req->segments[j]);
> + /* Check that there is no request in use */
> + list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
> + list_del(&req->free_list);
>
> - for (j = 0; j < MAX_INDIRECT_PAGES; j++)
> - kfree(req->indirect_pages[j]);
> + for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
> + kfree(req->segments[j]);
>
> - kfree(req);
> - i++;
> - }
> + for (j = 0; j < MAX_INDIRECT_PAGES; j++)
> + kfree(req->indirect_pages[j]);
>
> - WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
> + kfree(req);
> + i++;
> + }
> +
> + WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
> + }
> blkif->nr_ring_pages = 0;
>
> return 0;
> @@ -281,6 +314,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
> BUG_ON(!list_empty(&blkif->free_pages));
> BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
>
> + kfree(blkif->rings);
> kmem_cache_free(xen_blkif_cachep, blkif);
> }
>
> @@ -307,15 +341,19 @@ int __init xen_blkif_interface_init(void)
> struct xenbus_device *dev = to_xenbus_device(_dev); \
> struct backend_info *be = dev_get_drvdata(&dev->dev); \
> struct xen_blkif *blkif = be->blkif; \
> - struct xen_blkif_ring *ring = &blkif->ring; \
> + int i; \
> + \
> + for (i = 0; i < blkif->nr_rings; i++) { \
> + struct xen_blkif_ring *ring = &blkif->rings[i]; \
> \
> - blkif->st_oo_req = ring->st_oo_req; \
> - blkif->st_rd_req = ring->st_rd_req; \
> - blkif->st_wr_req = ring->st_wr_req; \
> - blkif->st_f_req = ring->st_f_req; \
> - blkif->st_ds_req = ring->st_ds_req; \
> - blkif->st_rd_sect = ring->st_rd_sect; \
> - blkif->st_wr_sect = ring->st_wr_sect; \
> + blkif->st_oo_req += ring->st_oo_req; \
> + blkif->st_rd_req += ring->st_rd_req; \
> + blkif->st_wr_req += ring->st_wr_req; \
> + blkif->st_f_req += ring->st_f_req; \
> + blkif->st_ds_req += ring->st_ds_req; \
> + blkif->st_rd_sect += ring->st_rd_sect; \
> + blkif->st_wr_sect += ring->st_wr_sect; \
> + } \
That needs fixing. You could alter the macro to only read the values
from the proper member.
> \
> return sprintf(buf, format, ##args); \
> } \
> @@ -439,6 +477,7 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
> static int xen_blkbk_remove(struct xenbus_device *dev)
> {
> struct backend_info *be = dev_get_drvdata(&dev->dev);
> + int i;
unsigned int
>
> pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
>
> @@ -455,7 +494,8 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
>
> if (be->blkif) {
> xen_blkif_disconnect(be->blkif);
> - xen_blkif_put(be->blkif);
> + for (i = 0; i < be->blkif->nr_rings; i++)
> + xen_blkif_put(be->blkif);
> }
>
> kfree(be->mode);
> @@ -839,20 +879,16 @@ again:
> }
>
>
> -static int connect_ring(struct backend_info *be)
> +static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
> {
> - struct xenbus_device *dev = be->dev;
> + unsigned int ring_page_order, nr_grefs, evtchn;
> unsigned int ring_ref[XENBUS_MAX_RING_PAGES];
> - unsigned int evtchn, nr_grefs, ring_page_order;
> - unsigned int pers_grants;
> - char protocol[64] = "";
> struct pending_req *req, *n;
> int err, i, j;
> - struct xen_blkif_ring *ring = &be->blkif->ring;
> -
> - pr_debug("%s %s\n", __func__, dev->otherend);
> + struct xen_blkif *blkif = ring->blkif;
> + struct xenbus_device *dev = blkif->be->dev;
>
> - err = xenbus_scanf(XBT_NIL, dev->otherend, "event-channel", "%u",
> + err = xenbus_scanf(XBT_NIL, dir, "event-channel", "%u",
> &evtchn);
> if (err != 1) {
> err = -EINVAL;
> @@ -860,12 +896,11 @@ static int connect_ring(struct backend_info *be)
> dev->otherend);
> return err;
> }
> - pr_info("event-channel %u\n", evtchn);
>
> err = xenbus_scanf(XBT_NIL, dev->otherend, "ring-page-order", "%u",
> &ring_page_order);
> if (err != 1) {
> - err = xenbus_scanf(XBT_NIL, dev->otherend, "ring-ref",
> + err = xenbus_scanf(XBT_NIL, dir, "ring-ref",
> "%u", &ring_ref[0]);
> if (err != 1) {
> err = -EINVAL;
> @@ -874,7 +909,7 @@ static int connect_ring(struct backend_info *be)
> return err;
> }
> nr_grefs = 1;
> - pr_info("%s:using single page: ring-ref %d\n", dev->otherend,
> + pr_info("%s:using single page: ring-ref %d\n", dir,
> ring_ref[0]);
> } else {
> unsigned int i;
> @@ -892,7 +927,7 @@ static int connect_ring(struct backend_info *be)
> char ring_ref_name[RINGREF_NAME_LEN];
>
> snprintf(ring_ref_name, RINGREF_NAME_LEN, "ring-ref%u", i);
> - err = xenbus_scanf(XBT_NIL, dev->otherend, ring_ref_name,
> + err = xenbus_scanf(XBT_NIL, dir, ring_ref_name,
> "%u", &ring_ref[i]);
> if (err != 1) {
> err = -EINVAL;
> @@ -900,38 +935,12 @@ static int connect_ring(struct backend_info *be)
> dev->otherend, ring_ref_name);
> return err;
> }
> - pr_info("ring-ref%u: %u\n", i, ring_ref[i]);
> + pr_info("%s: ring-ref%u: %u\n", dir, i, ring_ref[i]);
> }
> }
> + blkif->nr_ring_pages = nr_grefs;
>
> - be->blkif->blk_protocol = BLKIF_PROTOCOL_DEFAULT;
> - err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
> - "%63s", protocol, NULL);
> - if (err)
> - strcpy(protocol, "unspecified, assuming default");
> - else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
> - be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
> - else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
> - be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32;
> - else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64))
> - be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
> - else {
> - xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
> - return -1;
> - }
> - err = xenbus_gather(XBT_NIL, dev->otherend,
> - "feature-persistent", "%u",
> - &pers_grants, NULL);
> - if (err)
> - pers_grants = 0;
> -
> - be->blkif->vbd.feature_gnt_persistent = pers_grants;
> - be->blkif->vbd.overflow_max_grants = 0;
> - be->blkif->nr_ring_pages = nr_grefs;
> -
> - pr_info("ring-pages:%d, event-channel %d, protocol %d (%s) %s\n",
> - nr_grefs, evtchn, be->blkif->blk_protocol, protocol,
> - pers_grants ? "persistent grants" : "");
> + pr_info("ring-pages:%d, event-channel %d.\n", nr_grefs, evtchn);
>
> for (i = 0; i < nr_grefs * XEN_BLKIF_REQS_PER_PAGE; i++) {
> req = kzalloc(sizeof(*req), GFP_KERNEL);
> @@ -976,6 +985,71 @@ fail:
> kfree(req);
> }
> return -ENOMEM;
> +
> +}
> +
> +static int connect_ring(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + unsigned int pers_grants;
> + char protocol[64] = "";
> + int err, i;
> + char *xspath;
> + size_t xspathsize;
> + const size_t xenstore_path_ext_size = 11; /* sufficient for "/queue-NNN" */
> +
> + pr_debug("%s %s\n", __func__, dev->otherend);
> +
> + be->blkif->blk_protocol = BLKIF_PROTOCOL_DEFAULT;
> + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
> + "%63s", protocol, NULL);
> + if (err)
> + strcpy(protocol, "unspecified, assuming default");
> + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
> + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
> + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
> + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32;
> + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64))
> + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
> + else {
> + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
> + return -1;
> + }
> + err = xenbus_gather(XBT_NIL, dev->otherend,
> + "feature-persistent", "%u",
> + &pers_grants, NULL);
> + if (err)
> + pers_grants = 0;
> +
> + be->blkif->vbd.feature_gnt_persistent = pers_grants;
> + be->blkif->vbd.overflow_max_grants = 0;
> +
> + pr_info("nr_rings:%d protocol %d (%s) %s\n", be->blkif->nr_rings,
> + be->blkif->blk_protocol, protocol,
> + pers_grants ? "persistent grants" : "");
You need a bit more. Say the guest or something that would uniquely
identify it. Keep in mind there may be many guests launching at once.
> +
> + if (be->blkif->nr_rings == 1)
> + return read_per_ring_refs(&be->blkif->rings[0], dev->otherend);
> + else {
> + xspathsize = strlen(dev->otherend) + xenstore_path_ext_size;
> + xspath = kmalloc(xspathsize, GFP_KERNEL);
> + if (!xspath) {
> + xenbus_dev_fatal(dev, -ENOMEM, "reading ring references");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < be->blkif->nr_rings; i++) {
> + memset(xspath, 0, xspathsize);
> + snprintf(xspath, xspathsize, "%s/queue-%u", dev->otherend, i);
> + err = read_per_ring_refs(&be->blkif->rings[i], xspath);
> + if (err) {
> + kfree(xspath);
> + return err;
> + }
> + }
> + kfree(xspath);
> + }
> + return 0;
> }
>
> static const struct xenbus_device_id xen_blkbk_ids[] = {
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings
2015-11-05 2:30 ` Konrad Rzeszutek Wilk
@ 2015-11-05 3:02 ` Bob Liu
2015-11-05 3:24 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-05 3:02 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On 11/05/2015 10:30 AM, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 02, 2015 at 12:21:43PM +0800, Bob Liu wrote:
>> Preparatory patch for multiple hardware queues (rings). The number of
>> rings is unconditionally set to 1, larger number will be enabled in next
>> patch so as to make every single patch small and readable.
>
> Instead of saying 'next patch' - spell out the title of the patch.
>
>
>>
>> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>> ---
>> drivers/block/xen-blkback/common.h | 3 +-
>> drivers/block/xen-blkback/xenbus.c | 292 +++++++++++++++++++++++--------------
>> 2 files changed, 185 insertions(+), 110 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
>> index f0dd69a..4de1326 100644
>> --- a/drivers/block/xen-blkback/common.h
>> +++ b/drivers/block/xen-blkback/common.h
>> @@ -341,7 +341,8 @@ struct xen_blkif {
>> struct work_struct free_work;
>> unsigned int nr_ring_pages;
>> /* All rings for this device */
>> - struct xen_blkif_ring ring;
>> + struct xen_blkif_ring *rings;
>> + unsigned int nr_rings;
>> };
>>
>> struct seg_buf {
>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>> index 7bdd5fd..ac4b458 100644
>> --- a/drivers/block/xen-blkback/xenbus.c
>> +++ b/drivers/block/xen-blkback/xenbus.c
>> @@ -84,11 +84,12 @@ static int blkback_name(struct xen_blkif *blkif, char *buf)
>>
>> static void xen_update_blkif_status(struct xen_blkif *blkif)
>> {
>> - int err;
>> + int err, i;
>
> unsigned int.
>> char name[BLKBACK_NAME_LEN];
>> + struct xen_blkif_ring *ring;
>>
>> /* Not ready to connect? */
>> - if (!blkif->ring.irq || !blkif->vbd.bdev)
>> + if (!blkif->rings || !blkif->rings[0].irq || !blkif->vbd.bdev)
>> return;
>>
>> /* Already connected? */
>> @@ -113,19 +114,57 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
>> }
>> invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
>>
>> - blkif->ring.xenblkd = kthread_run(xen_blkif_schedule, &blkif->ring, "%s", name);
>> - if (IS_ERR(blkif->ring.xenblkd)) {
>> - err = PTR_ERR(blkif->ring.xenblkd);
>> - blkif->ring.xenblkd = NULL;
>> - xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
>> - return;
>> + if (blkif->nr_rings == 1) {
>> + blkif->rings[0].xenblkd = kthread_run(xen_blkif_schedule, &blkif->rings[0], "%s", name);
>> + if (IS_ERR(blkif->rings[0].xenblkd)) {
>> + err = PTR_ERR(blkif->rings[0].xenblkd);
>> + blkif->rings[0].xenblkd = NULL;
>> + xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
>> + return;
>> + }
>> + } else {
>> + for (i = 0; i < blkif->nr_rings; i++) {
>> + ring = &blkif->rings[i];
>> + ring->xenblkd = kthread_run(xen_blkif_schedule, ring, "%s-%d", name, i);
>> + if (IS_ERR(ring->xenblkd)) {
>> + err = PTR_ERR(ring->xenblkd);
>> + ring->xenblkd = NULL;
>> + xenbus_dev_error(blkif->be->dev, err,
>> + "start %s-%d xenblkd", name, i);
>> + return;
>> + }
>> + }
>
> Please collapse this together and just have one loop.
>
> And just use the same name throughout ('%s-%d')
>
> Also this does not take care of actually freeing the allocated
> ring->xenblkd if one of them fails to start.
>
> Hmm, looking at this function .. we seem to forget to change the
> state if something fails.
>
> As in, connect switches the state to Connected.. And if anything blows
> up after the connect() we don't switch the state. We do report an error
> in the XenBus, but the state is the same.
>
> We should be using xenbus_dev_fatal I believe - so at least the state
> changes to Closing (and then the frontend can switch itself to
> Closing->Closed) - and we will call 'xen_blkif_disconnect' on Closed.
>
Will update..
>> + }
>> +}
>> +
>> +static int xen_blkif_alloc_rings(struct xen_blkif *blkif)
>> +{
>> + int r;
>
> unsigned int i;
>
>> +
>> + blkif->rings = kzalloc(blkif->nr_rings * sizeof(struct xen_blkif_ring), GFP_KERNEL);
>> + if (!blkif->rings)
>> + return -ENOMEM;
>> +
>> + for (r = 0; r < blkif->nr_rings; r++) {
>> + struct xen_blkif_ring *ring = &blkif->rings[r];
>> +
>> + spin_lock_init(&ring->blk_ring_lock);
>> + init_waitqueue_head(&ring->wq);
>> + INIT_LIST_HEAD(&ring->pending_free);
>> +
>> + spin_lock_init(&ring->pending_free_lock);
>> + init_waitqueue_head(&ring->pending_free_wq);
>> + init_waitqueue_head(&ring->shutdown_wq);
>> + ring->blkif = blkif;
>> + xen_blkif_get(blkif);
>> }
>> +
>> + return 0;
>> }
>>
>> static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>> {
>> struct xen_blkif *blkif;
>> - struct xen_blkif_ring *ring;
>>
>> BUILD_BUG_ON(MAX_INDIRECT_PAGES > BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
>>
>> @@ -136,27 +175,17 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>> blkif->domid = domid;
>> atomic_set(&blkif->refcnt, 1);
>> init_completion(&blkif->drain_complete);
>> - atomic_set(&blkif->drain, 0);
>> INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>> spin_lock_init(&blkif->free_pages_lock);
>> INIT_LIST_HEAD(&blkif->free_pages);
>> - blkif->free_pages_num = 0;
>> - blkif->persistent_gnts.rb_node = NULL;
>> INIT_LIST_HEAD(&blkif->persistent_purge_list);
>> - atomic_set(&blkif->persistent_gnt_in_use, 0);
>> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>>
>> - ring = &blkif->ring;
>> - ring->blkif = blkif;
>> - spin_lock_init(&ring->blk_ring_lock);
>> - init_waitqueue_head(&ring->wq);
>> - ring->st_print = jiffies;
>> - atomic_set(&ring->inflight, 0);
>> -
>> - INIT_LIST_HEAD(&ring->pending_free);
>> - spin_lock_init(&ring->pending_free_lock);
>> - init_waitqueue_head(&ring->pending_free_wq);
>> - init_waitqueue_head(&ring->shutdown_wq);
>> + blkif->nr_rings = 1;
>> + if (xen_blkif_alloc_rings(blkif)) {
>> + kmem_cache_free(xen_blkif_cachep, blkif);
>> + return ERR_PTR(-ENOMEM);
>> + }
>>
>> return blkif;
>> }
>> @@ -218,50 +247,54 @@ static int xen_blkif_map(struct xen_blkif_ring *ring, grant_ref_t *gref,
>> static int xen_blkif_disconnect(struct xen_blkif *blkif)
>> {
>> struct pending_req *req, *n;
>> - int i = 0, j;
>> - struct xen_blkif_ring *ring = &blkif->ring;
>> + int j, r;
>>
>
> unsigned int i;
>> - if (ring->xenblkd) {
>> - kthread_stop(ring->xenblkd);
>> - wake_up(&ring->shutdown_wq);
>> - ring->xenblkd = NULL;
>> - }
>> + for (r = 0; r < blkif->nr_rings; r++) {
>> + struct xen_blkif_ring *ring = &blkif->rings[r];
>> + int i = 0;
>
> unsigned int
>>
>> - /* The above kthread_stop() guarantees that at this point we
>> - * don't have any discard_io or other_io requests. So, checking
>> - * for inflight IO is enough.
>> - */
>> - if (atomic_read(&ring->inflight) > 0)
>> - return -EBUSY;
>> + if (ring->xenblkd) {
>> + kthread_stop(ring->xenblkd);
>> + wake_up(&ring->shutdown_wq);
>> + ring->xenblkd = NULL;
>> + }
>>
>> - if (ring->irq) {
>> - unbind_from_irqhandler(ring->irq, ring);
>> - ring->irq = 0;
>> - }
>> + /* The above kthread_stop() guarantees that at this point we
>> + * don't have any discard_io or other_io requests. So, checking
>> + * for inflight IO is enough.
>> + */
>> + if (atomic_read(&ring->inflight) > 0)
>> + return -EBUSY;
>>
>> - if (ring->blk_rings.common.sring) {
>> - xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
>> - ring->blk_rings.common.sring = NULL;
>> - }
>> + if (ring->irq) {
>> + unbind_from_irqhandler(ring->irq, ring);
>> + ring->irq = 0;
>> + }
>>
>> - /* Remove all persistent grants and the cache of ballooned pages. */
>> - xen_blkbk_free_caches(ring);
>> + if (ring->blk_rings.common.sring) {
>> + xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
>> + ring->blk_rings.common.sring = NULL;
>> + }
>>
>> - /* Check that there is no request in use */
>> - list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
>> - list_del(&req->free_list);
>> + /* Remove all persistent grants and the cache of ballooned pages. */
>> + xen_blkbk_free_caches(ring);
>>
>> - for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
>> - kfree(req->segments[j]);
>> + /* Check that there is no request in use */
>> + list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
>> + list_del(&req->free_list);
>>
>> - for (j = 0; j < MAX_INDIRECT_PAGES; j++)
>> - kfree(req->indirect_pages[j]);
>> + for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
>> + kfree(req->segments[j]);
>>
>> - kfree(req);
>> - i++;
>> - }
>> + for (j = 0; j < MAX_INDIRECT_PAGES; j++)
>> + kfree(req->indirect_pages[j]);
>>
>> - WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
>> + kfree(req);
>> + i++;
>> + }
>> +
>> + WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
>> + }
>> blkif->nr_ring_pages = 0;
>>
>> return 0;
>> @@ -281,6 +314,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>> BUG_ON(!list_empty(&blkif->free_pages));
>> BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
>>
>> + kfree(blkif->rings);
>> kmem_cache_free(xen_blkif_cachep, blkif);
>> }
>>
>> @@ -307,15 +341,19 @@ int __init xen_blkif_interface_init(void)
>> struct xenbus_device *dev = to_xenbus_device(_dev); \
>> struct backend_info *be = dev_get_drvdata(&dev->dev); \
>> struct xen_blkif *blkif = be->blkif; \
>> - struct xen_blkif_ring *ring = &blkif->ring; \
>> + int i; \
>> + \
>> + for (i = 0; i < blkif->nr_rings; i++) { \
>> + struct xen_blkif_ring *ring = &blkif->rings[i]; \
>> \
>> - blkif->st_oo_req = ring->st_oo_req; \
>> - blkif->st_rd_req = ring->st_rd_req; \
>> - blkif->st_wr_req = ring->st_wr_req; \
>> - blkif->st_f_req = ring->st_f_req; \
>> - blkif->st_ds_req = ring->st_ds_req; \
>> - blkif->st_rd_sect = ring->st_rd_sect; \
>> - blkif->st_wr_sect = ring->st_wr_sect; \
>> + blkif->st_oo_req += ring->st_oo_req; \
>> + blkif->st_rd_req += ring->st_rd_req; \
>> + blkif->st_wr_req += ring->st_wr_req; \
>> + blkif->st_f_req += ring->st_f_req; \
>> + blkif->st_ds_req += ring->st_ds_req; \
>> + blkif->st_rd_sect += ring->st_rd_sect; \
>> + blkif->st_wr_sect += ring->st_wr_sect; \
>> + } \
>
> That needs fixing. You could alter the macro to only read the values
> from the proper member.
Do you think we still need these debug values for per-ring?
I'm thinking of just leave them for per-device(blkif), but that requires extra lock to protect?
--
Regards,
-Bob
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings
2015-11-05 3:02 ` Bob Liu
@ 2015-11-05 3:24 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 3:24 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On November 4, 2015 10:02:26 PM EST, Bob Liu <bob.liu@oracle.com> wrote:
>
>On 11/05/2015 10:30 AM, Konrad Rzeszutek Wilk wrote:
>> On Mon, Nov 02, 2015 at 12:21:43PM +0800, Bob Liu wrote:
>>> Preparatory patch for multiple hardware queues (rings). The number
>of
>>> rings is unconditionally set to 1, larger number will be enabled in
>next
>>> patch so as to make every single patch small and readable.
>>
>> Instead of saying 'next patch' - spell out the title of the patch.
>>
>>
>>>
>>> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
>>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>>> ---
>>> drivers/block/xen-blkback/common.h | 3 +-
>>> drivers/block/xen-blkback/xenbus.c | 292
>+++++++++++++++++++++++--------------
>>> 2 files changed, 185 insertions(+), 110 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkback/common.h
>b/drivers/block/xen-blkback/common.h
>>> index f0dd69a..4de1326 100644
>>> --- a/drivers/block/xen-blkback/common.h
>>> +++ b/drivers/block/xen-blkback/common.h
>>> @@ -341,7 +341,8 @@ struct xen_blkif {
>>> struct work_struct free_work;
>>> unsigned int nr_ring_pages;
>>> /* All rings for this device */
>>> - struct xen_blkif_ring ring;
>>> + struct xen_blkif_ring *rings;
>>> + unsigned int nr_rings;
>>> };
>>>
>>> struct seg_buf {
>>> diff --git a/drivers/block/xen-blkback/xenbus.c
>b/drivers/block/xen-blkback/xenbus.c
>>> index 7bdd5fd..ac4b458 100644
>>> --- a/drivers/block/xen-blkback/xenbus.c
>>> +++ b/drivers/block/xen-blkback/xenbus.c
>>> @@ -84,11 +84,12 @@ static int blkback_name(struct xen_blkif *blkif,
>char *buf)
>>>
>>> static void xen_update_blkif_status(struct xen_blkif *blkif)
>>> {
>>> - int err;
>>> + int err, i;
>>
>> unsigned int.
>>> char name[BLKBACK_NAME_LEN];
>>> + struct xen_blkif_ring *ring;
>>>
>>> /* Not ready to connect? */
>>> - if (!blkif->ring.irq || !blkif->vbd.bdev)
>>> + if (!blkif->rings || !blkif->rings[0].irq || !blkif->vbd.bdev)
>>> return;
>>>
>>> /* Already connected? */
>>> @@ -113,19 +114,57 @@ static void xen_update_blkif_status(struct
>xen_blkif *blkif)
>>> }
>>> invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
>>>
>>> - blkif->ring.xenblkd = kthread_run(xen_blkif_schedule,
>&blkif->ring, "%s", name);
>>> - if (IS_ERR(blkif->ring.xenblkd)) {
>>> - err = PTR_ERR(blkif->ring.xenblkd);
>>> - blkif->ring.xenblkd = NULL;
>>> - xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
>>> - return;
>>> + if (blkif->nr_rings == 1) {
>>> + blkif->rings[0].xenblkd = kthread_run(xen_blkif_schedule,
>&blkif->rings[0], "%s", name);
>>> + if (IS_ERR(blkif->rings[0].xenblkd)) {
>>> + err = PTR_ERR(blkif->rings[0].xenblkd);
>>> + blkif->rings[0].xenblkd = NULL;
>>> + xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
>>> + return;
>>> + }
>>> + } else {
>>> + for (i = 0; i < blkif->nr_rings; i++) {
>>> + ring = &blkif->rings[i];
>>> + ring->xenblkd = kthread_run(xen_blkif_schedule, ring, "%s-%d",
>name, i);
>>> + if (IS_ERR(ring->xenblkd)) {
>>> + err = PTR_ERR(ring->xenblkd);
>>> + ring->xenblkd = NULL;
>>> + xenbus_dev_error(blkif->be->dev, err,
>>> + "start %s-%d xenblkd", name, i);
>>> + return;
>>> + }
>>> + }
>>
>> Please collapse this together and just have one loop.
>>
>> And just use the same name throughout ('%s-%d')
>>
>> Also this does not take care of actually freeing the allocated
>> ring->xenblkd if one of them fails to start.
>>
>> Hmm, looking at this function .. we seem to forget to change the
>> state if something fails.
>>
>> As in, connect switches the state to Connected.. And if anything
>blows
>> up after the connect() we don't switch the state. We do report an
>error
>> in the XenBus, but the state is the same.
>>
>> We should be using xenbus_dev_fatal I believe - so at least the state
>> changes to Closing (and then the frontend can switch itself to
>> Closing->Closed) - and we will call 'xen_blkif_disconnect' on Closed.
>
>>
>
>Will update..
>
>>> + }
>>> +}
>>> +
>>> +static int xen_blkif_alloc_rings(struct xen_blkif *blkif)
>>> +{
>>> + int r;
>>
>> unsigned int i;
>>
>>> +
>>> + blkif->rings = kzalloc(blkif->nr_rings * sizeof(struct
>xen_blkif_ring), GFP_KERNEL);
>>> + if (!blkif->rings)
>>> + return -ENOMEM;
>>> +
>>> + for (r = 0; r < blkif->nr_rings; r++) {
>>> + struct xen_blkif_ring *ring = &blkif->rings[r];
>>> +
>>> + spin_lock_init(&ring->blk_ring_lock);
>>> + init_waitqueue_head(&ring->wq);
>>> + INIT_LIST_HEAD(&ring->pending_free);
>>> +
>>> + spin_lock_init(&ring->pending_free_lock);
>>> + init_waitqueue_head(&ring->pending_free_wq);
>>> + init_waitqueue_head(&ring->shutdown_wq);
>>> + ring->blkif = blkif;
>>> + xen_blkif_get(blkif);
>>> }
>>> +
>>> + return 0;
>>> }
>>>
>>> static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>>> {
>>> struct xen_blkif *blkif;
>>> - struct xen_blkif_ring *ring;
>>>
>>> BUILD_BUG_ON(MAX_INDIRECT_PAGES >
>BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST);
>>>
>>> @@ -136,27 +175,17 @@ static struct xen_blkif
>*xen_blkif_alloc(domid_t domid)
>>> blkif->domid = domid;
>>> atomic_set(&blkif->refcnt, 1);
>>> init_completion(&blkif->drain_complete);
>>> - atomic_set(&blkif->drain, 0);
>>> INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>>> spin_lock_init(&blkif->free_pages_lock);
>>> INIT_LIST_HEAD(&blkif->free_pages);
>>> - blkif->free_pages_num = 0;
>>> - blkif->persistent_gnts.rb_node = NULL;
>>> INIT_LIST_HEAD(&blkif->persistent_purge_list);
>>> - atomic_set(&blkif->persistent_gnt_in_use, 0);
>>> INIT_WORK(&blkif->persistent_purge_work,
>xen_blkbk_unmap_purged_grants);
>>>
>>> - ring = &blkif->ring;
>>> - ring->blkif = blkif;
>>> - spin_lock_init(&ring->blk_ring_lock);
>>> - init_waitqueue_head(&ring->wq);
>>> - ring->st_print = jiffies;
>>> - atomic_set(&ring->inflight, 0);
>>> -
>>> - INIT_LIST_HEAD(&ring->pending_free);
>>> - spin_lock_init(&ring->pending_free_lock);
>>> - init_waitqueue_head(&ring->pending_free_wq);
>>> - init_waitqueue_head(&ring->shutdown_wq);
>>> + blkif->nr_rings = 1;
>>> + if (xen_blkif_alloc_rings(blkif)) {
>>> + kmem_cache_free(xen_blkif_cachep, blkif);
>>> + return ERR_PTR(-ENOMEM);
>>> + }
>>>
>>> return blkif;
>>> }
>>> @@ -218,50 +247,54 @@ static int xen_blkif_map(struct xen_blkif_ring
>*ring, grant_ref_t *gref,
>>> static int xen_blkif_disconnect(struct xen_blkif *blkif)
>>> {
>>> struct pending_req *req, *n;
>>> - int i = 0, j;
>>> - struct xen_blkif_ring *ring = &blkif->ring;
>>> + int j, r;
>>>
>>
>> unsigned int i;
>>> - if (ring->xenblkd) {
>>> - kthread_stop(ring->xenblkd);
>>> - wake_up(&ring->shutdown_wq);
>>> - ring->xenblkd = NULL;
>>> - }
>>> + for (r = 0; r < blkif->nr_rings; r++) {
>>> + struct xen_blkif_ring *ring = &blkif->rings[r];
>>> + int i = 0;
>>
>> unsigned int
>>>
>>> - /* The above kthread_stop() guarantees that at this point we
>>> - * don't have any discard_io or other_io requests. So, checking
>>> - * for inflight IO is enough.
>>> - */
>>> - if (atomic_read(&ring->inflight) > 0)
>>> - return -EBUSY;
>>> + if (ring->xenblkd) {
>>> + kthread_stop(ring->xenblkd);
>>> + wake_up(&ring->shutdown_wq);
>>> + ring->xenblkd = NULL;
>>> + }
>>>
>>> - if (ring->irq) {
>>> - unbind_from_irqhandler(ring->irq, ring);
>>> - ring->irq = 0;
>>> - }
>>> + /* The above kthread_stop() guarantees that at this point we
>>> + * don't have any discard_io or other_io requests. So, checking
>>> + * for inflight IO is enough.
>>> + */
>>> + if (atomic_read(&ring->inflight) > 0)
>>> + return -EBUSY;
>>>
>>> - if (ring->blk_rings.common.sring) {
>>> - xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
>>> - ring->blk_rings.common.sring = NULL;
>>> - }
>>> + if (ring->irq) {
>>> + unbind_from_irqhandler(ring->irq, ring);
>>> + ring->irq = 0;
>>> + }
>>>
>>> - /* Remove all persistent grants and the cache of ballooned pages.
>*/
>>> - xen_blkbk_free_caches(ring);
>>> + if (ring->blk_rings.common.sring) {
>>> + xenbus_unmap_ring_vfree(blkif->be->dev, ring->blk_ring);
>>> + ring->blk_rings.common.sring = NULL;
>>> + }
>>>
>>> - /* Check that there is no request in use */
>>> - list_for_each_entry_safe(req, n, &ring->pending_free, free_list) {
>>> - list_del(&req->free_list);
>>> + /* Remove all persistent grants and the cache of ballooned pages.
>*/
>>> + xen_blkbk_free_caches(ring);
>>>
>>> - for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
>>> - kfree(req->segments[j]);
>>> + /* Check that there is no request in use */
>>> + list_for_each_entry_safe(req, n, &ring->pending_free, free_list)
>{
>>> + list_del(&req->free_list);
>>>
>>> - for (j = 0; j < MAX_INDIRECT_PAGES; j++)
>>> - kfree(req->indirect_pages[j]);
>>> + for (j = 0; j < MAX_INDIRECT_SEGMENTS; j++)
>>> + kfree(req->segments[j]);
>>>
>>> - kfree(req);
>>> - i++;
>>> - }
>>> + for (j = 0; j < MAX_INDIRECT_PAGES; j++)
>>> + kfree(req->indirect_pages[j]);
>>>
>>> - WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
>>> + kfree(req);
>>> + i++;
>>> + }
>>> +
>>> + WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
>>> + }
>>> blkif->nr_ring_pages = 0;
>>>
>>> return 0;
>>> @@ -281,6 +314,7 @@ static void xen_blkif_free(struct xen_blkif
>*blkif)
>>> BUG_ON(!list_empty(&blkif->free_pages));
>>> BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
>>>
>>> + kfree(blkif->rings);
>>> kmem_cache_free(xen_blkif_cachep, blkif);
>>> }
>>>
>>> @@ -307,15 +341,19 @@ int __init xen_blkif_interface_init(void)
>>> struct xenbus_device *dev = to_xenbus_device(_dev); \
>>> struct backend_info *be = dev_get_drvdata(&dev->dev); \
>>> struct xen_blkif *blkif = be->blkif; \
>>> - struct xen_blkif_ring *ring = &blkif->ring; \
>>> + int i; \
>>> + \
>>> + for (i = 0; i < blkif->nr_rings; i++) { \
>>> + struct xen_blkif_ring *ring = &blkif->rings[i]; \
>>> \
>>> - blkif->st_oo_req = ring->st_oo_req; \
>>> - blkif->st_rd_req = ring->st_rd_req; \
>>> - blkif->st_wr_req = ring->st_wr_req; \
>>> - blkif->st_f_req = ring->st_f_req; \
>>> - blkif->st_ds_req = ring->st_ds_req; \
>>> - blkif->st_rd_sect = ring->st_rd_sect; \
>>> - blkif->st_wr_sect = ring->st_wr_sect; \
>>> + blkif->st_oo_req += ring->st_oo_req; \
>>> + blkif->st_rd_req += ring->st_rd_req; \
>>> + blkif->st_wr_req += ring->st_wr_req; \
>>> + blkif->st_f_req += ring->st_f_req; \
>>> + blkif->st_ds_req += ring->st_ds_req; \
>>> + blkif->st_rd_sect += ring->st_rd_sect; \
>>> + blkif->st_wr_sect += ring->st_wr_sect; \
>>> + } \
>>
>> That needs fixing. You could alter the macro to only read the values
>> from the proper member.
>
>Do you think we still need these debug values for per-ring?
>I'm thinking of just leave them for per-device(blkif), but that
>requires extra lock to protect?
It should be per device. However I think you are OK with locking - as long as the sysfs is torn down before we deallocate the rings.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 08/10] xen/blkback: get the number of hardware queues/rings from blkfront
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (6 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 07/10] xen/blkback: pseudo support for multi hardware queues/rings Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-05 2:37 ` Konrad Rzeszutek Wilk
2015-11-02 4:21 ` [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue Bob Liu
` (2 subsequent siblings)
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Backend advertises "multi-queue-max-queues" to front, then get the negotiated
number from "multi-queue-num-queues" wrote by blkfront.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkback/blkback.c | 11 +++++++++++
drivers/block/xen-blkback/common.h | 1 +
drivers/block/xen-blkback/xenbus.c | 35 +++++++++++++++++++++++++++++------
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index eaf7ec0..107cc4a 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -83,6 +83,11 @@ module_param_named(max_persistent_grants, xen_blkif_max_pgrants, int, 0644);
MODULE_PARM_DESC(max_persistent_grants,
"Maximum number of grants to map persistently");
+unsigned int xenblk_max_queues;
+module_param_named(max_queues, xenblk_max_queues, uint, 0644);
+MODULE_PARM_DESC(max_queues,
+ "Maximum number of hardware queues per virtual disk");
+
/*
* Maximum order of pages to be used for the shared ring between front and
* backend, 4KB page granularity is used.
@@ -1478,6 +1483,12 @@ static int __init xen_blkif_init(void)
xen_blkif_max_ring_order = XENBUS_MAX_RING_PAGE_ORDER;
}
+ /* Allow as many queues as there are CPUs if user has not
+ * specified a value.
+ */
+ if (xenblk_max_queues == 0)
+ xenblk_max_queues = num_online_cpus();
+
rc = xen_blkif_interface_init();
if (rc)
goto failed_init;
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 4de1326..fb28b91 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -45,6 +45,7 @@
#include <xen/interface/io/protocols.h>
extern unsigned int xen_blkif_max_ring_order;
+extern unsigned int xenblk_max_queues;
/*
* This is the maximum number of segments that would be allowed in indirect
* requests. This value will also be passed to the frontend.
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index ac4b458..cafbadd 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -181,12 +181,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
INIT_LIST_HEAD(&blkif->persistent_purge_list);
INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
- blkif->nr_rings = 1;
- if (xen_blkif_alloc_rings(blkif)) {
- kmem_cache_free(xen_blkif_cachep, blkif);
- return ERR_PTR(-ENOMEM);
- }
-
return blkif;
}
@@ -606,6 +600,14 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
goto fail;
}
+ /* Multi-queue: write how many queues are supported by the backend. */
+ err = xenbus_printf(XBT_NIL, dev->nodename,
+ "multi-queue-max-queues", "%u", xenblk_max_queues);
+ if (err) {
+ pr_warn("Error writing multi-queue-num-queues\n");
+ goto fail;
+ }
+
/* setup back pointer */
be->blkif->be = be;
@@ -997,6 +999,7 @@ static int connect_ring(struct backend_info *be)
char *xspath;
size_t xspathsize;
const size_t xenstore_path_ext_size = 11; /* sufficient for "/queue-NNN" */
+ unsigned int requested_num_queues = 0;
pr_debug("%s %s\n", __func__, dev->otherend);
@@ -1024,6 +1027,26 @@ static int connect_ring(struct backend_info *be)
be->blkif->vbd.feature_gnt_persistent = pers_grants;
be->blkif->vbd.overflow_max_grants = 0;
+ /*
+ * Read the number of hardware queues from frontend.
+ */
+ err = xenbus_scanf(XBT_NIL, dev->otherend, "multi-queue-num-queues", "%u", &requested_num_queues);
+ if (err < 0) {
+ requested_num_queues = 1;
+ } else {
+ if (requested_num_queues > xenblk_max_queues
+ || requested_num_queues == 0) {
+ /* buggy or malicious guest */
+ xenbus_dev_fatal(dev, err,
+ "guest requested %u queues, exceeding the maximum of %u.",
+ requested_num_queues, xenblk_max_queues);
+ return -1;
+ }
+ }
+ be->blkif->nr_rings = requested_num_queues;
+ if (xen_blkif_alloc_rings(be->blkif))
+ return -ENOMEM;
+
pr_info("nr_rings:%d protocol %d (%s) %s\n", be->blkif->nr_rings,
be->blkif->blk_protocol, protocol,
pers_grants ? "persistent grants" : "");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 08/10] xen/blkback: get the number of hardware queues/rings from blkfront
2015-11-02 4:21 ` [PATCH v4 08/10] xen/blkback: get the number of hardware queues/rings from blkfront Bob Liu
@ 2015-11-05 2:37 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 2:37 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On Mon, Nov 02, 2015 at 12:21:44PM +0800, Bob Liu wrote:
> Backend advertises "multi-queue-max-queues" to front, then get the negotiated
s/then/so/
> number from "multi-queue-num-queues" wrote by blkfront.
s/wrote/written/
>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> ---
> drivers/block/xen-blkback/blkback.c | 11 +++++++++++
> drivers/block/xen-blkback/common.h | 1 +
> drivers/block/xen-blkback/xenbus.c | 35 +++++++++++++++++++++++++++++------
> 3 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index eaf7ec0..107cc4a 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -83,6 +83,11 @@ module_param_named(max_persistent_grants, xen_blkif_max_pgrants, int, 0644);
> MODULE_PARM_DESC(max_persistent_grants,
> "Maximum number of grants to map persistently");
>
> +unsigned int xenblk_max_queues;
Do you want a default value?
> +module_param_named(max_queues, xenblk_max_queues, uint, 0644);
> +MODULE_PARM_DESC(max_queues,
> + "Maximum number of hardware queues per virtual disk");
> +
> /*
> * Maximum order of pages to be used for the shared ring between front and
> * backend, 4KB page granularity is used.
> @@ -1478,6 +1483,12 @@ static int __init xen_blkif_init(void)
> xen_blkif_max_ring_order = XENBUS_MAX_RING_PAGE_ORDER;
> }
>
> + /* Allow as many queues as there are CPUs if user has not
> + * specified a value.
That should be part of the module_param actually.
> + */
> + if (xenblk_max_queues == 0)
> + xenblk_max_queues = num_online_cpus();
> +
> rc = xen_blkif_interface_init();
> if (rc)
> goto failed_init;
> diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
> index 4de1326..fb28b91 100644
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -45,6 +45,7 @@
> #include <xen/interface/io/protocols.h>
>
> extern unsigned int xen_blkif_max_ring_order;
> +extern unsigned int xenblk_max_queues;
> /*
> * This is the maximum number of segments that would be allowed in indirect
> * requests. This value will also be passed to the frontend.
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index ac4b458..cafbadd 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -181,12 +181,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
> INIT_LIST_HEAD(&blkif->persistent_purge_list);
> INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
>
> - blkif->nr_rings = 1;
> - if (xen_blkif_alloc_rings(blkif)) {
> - kmem_cache_free(xen_blkif_cachep, blkif);
> - return ERR_PTR(-ENOMEM);
> - }
> -
> return blkif;
> }
>
> @@ -606,6 +600,14 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
> goto fail;
> }
>
> + /* Multi-queue: write how many queues are supported by the backend. */
> + err = xenbus_printf(XBT_NIL, dev->nodename,
> + "multi-queue-max-queues", "%u", xenblk_max_queues);
> + if (err) {
> + pr_warn("Error writing multi-queue-num-queues\n");
> + goto fail;
Why fail? If we can't - then we can't and won't advertise this support.
We should still be able to work, right?
That is how the ''max-ring-page-order' does it below.
> + }
> +
> /* setup back pointer */
> be->blkif->be = be;
>
> @@ -997,6 +999,7 @@ static int connect_ring(struct backend_info *be)
> char *xspath;
> size_t xspathsize;
> const size_t xenstore_path_ext_size = 11; /* sufficient for "/queue-NNN" */
> + unsigned int requested_num_queues = 0;
>
> pr_debug("%s %s\n", __func__, dev->otherend);
>
> @@ -1024,6 +1027,26 @@ static int connect_ring(struct backend_info *be)
> be->blkif->vbd.feature_gnt_persistent = pers_grants;
> be->blkif->vbd.overflow_max_grants = 0;
>
> + /*
> + * Read the number of hardware queues from frontend.
> + */
> + err = xenbus_scanf(XBT_NIL, dev->otherend, "multi-queue-num-queues", "%u", &requested_num_queues);
Please split this in two lines.
> + if (err < 0) {
> + requested_num_queues = 1;
> + } else {
> + if (requested_num_queues > xenblk_max_queues
> + || requested_num_queues == 0) {
> + /* buggy or malicious guest */
> + xenbus_dev_fatal(dev, err,
> + "guest requested %u queues, exceeding the maximum of %u.",
> + requested_num_queues, xenblk_max_queues);
> + return -1;
> + }
> + }
> + be->blkif->nr_rings = requested_num_queues;
> + if (xen_blkif_alloc_rings(be->blkif))
> + return -ENOMEM;
> +
> pr_info("nr_rings:%d protocol %d (%s) %s\n", be->blkif->nr_rings,
> be->blkif->blk_protocol, protocol,
> pers_grants ? "persistent grants" : "");
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (7 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 08/10] xen/blkback: get the number of hardware queues/rings from blkfront Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-05 2:39 ` Konrad Rzeszutek Wilk
2015-11-02 4:21 ` [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue Bob Liu
2015-11-02 11:19 ` [Xen-devel] [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Julien Grall
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Make persistent grants per-queue/ring instead of per-device, so that we can
drop the 'dev_lock' and get better scalability.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkfront.c | 89 +++++++++++++++++---------------------------
1 file changed, 34 insertions(+), 55 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 23096d7..eb19f08 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -133,6 +133,8 @@ struct blkfront_ring_info {
struct gnttab_free_callback callback;
struct blk_shadow shadow[BLK_MAX_RING_SIZE];
struct list_head indirect_pages;
+ struct list_head grants;
+ unsigned int persistent_gnts_c;
unsigned long shadow_free;
struct blkfront_info *dev_info;
};
@@ -144,8 +146,6 @@ struct blkfront_ring_info {
*/
struct blkfront_info
{
- /* Lock to proect info->grants list shared by multi rings */
- spinlock_t dev_lock;
struct mutex mutex;
struct xenbus_device *xbdev;
struct gendisk *gd;
@@ -155,8 +155,6 @@ struct blkfront_info
/* Number of pages per ring buffer */
unsigned int nr_ring_pages;
struct request_queue *rq;
- struct list_head grants;
- unsigned int persistent_gnts_c;
unsigned int feature_flush;
unsigned int feature_discard:1;
unsigned int feature_secdiscard:1;
@@ -231,7 +229,6 @@ static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
struct grant *gnt_list_entry, *n;
int i = 0;
- spin_lock_irq(&info->dev_lock);
while(i < num) {
gnt_list_entry = kzalloc(sizeof(struct grant), GFP_NOIO);
if (!gnt_list_entry)
@@ -247,35 +244,32 @@ static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
}
gnt_list_entry->gref = GRANT_INVALID_REF;
- list_add(&gnt_list_entry->node, &info->grants);
+ list_add(&gnt_list_entry->node, &rinfo->grants);
i++;
}
- spin_unlock_irq(&info->dev_lock);
return 0;
out_of_memory:
list_for_each_entry_safe(gnt_list_entry, n,
- &info->grants, node) {
+ &rinfo->grants, node) {
list_del(&gnt_list_entry->node);
if (info->feature_persistent)
__free_page(pfn_to_page(gnt_list_entry->pfn));
kfree(gnt_list_entry);
i--;
}
- spin_unlock_irq(&info->dev_lock);
BUG_ON(i != 0);
return -ENOMEM;
}
static struct grant *get_grant(grant_ref_t *gref_head,
unsigned long pfn,
- struct blkfront_info *info)
+ struct blkfront_ring_info *info)
{
struct grant *gnt_list_entry;
unsigned long buffer_gfn;
- spin_lock(&info->dev_lock);
BUG_ON(list_empty(&info->grants));
gnt_list_entry = list_first_entry(&info->grants, struct grant,
node);
@@ -283,21 +277,19 @@ static struct grant *get_grant(grant_ref_t *gref_head,
if (gnt_list_entry->gref != GRANT_INVALID_REF) {
info->persistent_gnts_c--;
- spin_unlock(&info->dev_lock);
return gnt_list_entry;
}
- spin_unlock(&info->dev_lock);
/* Assign a gref to this page */
gnt_list_entry->gref = gnttab_claim_grant_reference(gref_head);
BUG_ON(gnt_list_entry->gref == -ENOSPC);
- if (!info->feature_persistent) {
+ if (!info->dev_info->feature_persistent) {
BUG_ON(!pfn);
gnt_list_entry->pfn = pfn;
}
buffer_gfn = pfn_to_gfn(gnt_list_entry->pfn);
gnttab_grant_foreign_access_ref(gnt_list_entry->gref,
- info->xbdev->otherend_id,
+ info->dev_info->xbdev->otherend_id,
buffer_gfn, 0);
return gnt_list_entry;
}
@@ -559,13 +551,13 @@ static int blkif_queue_request(struct request *req, struct blkfront_ring_info *r
list_del(&indirect_page->lru);
pfn = page_to_pfn(indirect_page);
}
- gnt_list_entry = get_grant(&gref_head, pfn, info);
+ gnt_list_entry = get_grant(&gref_head, pfn, rinfo);
rinfo->shadow[id].indirect_grants[n] = gnt_list_entry;
segments = kmap_atomic(pfn_to_page(gnt_list_entry->pfn));
ring_req->u.indirect.indirect_grefs[n] = gnt_list_entry->gref;
}
- gnt_list_entry = get_grant(&gref_head, page_to_pfn(sg_page(sg)), info);
+ gnt_list_entry = get_grant(&gref_head, page_to_pfn(sg_page(sg)), rinfo);
ref = gnt_list_entry->gref;
rinfo->shadow[id].grants_used[i] = gnt_list_entry;
@@ -992,7 +984,7 @@ static void blkif_restart_queue(struct work_struct *work)
static void blkif_free_ring(struct blkfront_ring_info *rinfo)
{
- struct grant *persistent_gnt;
+ struct grant *persistent_gnt, *n;
struct blkfront_info *info = rinfo->dev_info;
int i, j, segs;
@@ -1010,6 +1002,23 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
}
}
+ /* Remove all persistent grants */
+ if (!list_empty(&rinfo->grants)) {
+ list_for_each_entry_safe(persistent_gnt, n,
+ &rinfo->grants, node) {
+ list_del(&persistent_gnt->node);
+ if (persistent_gnt->gref != GRANT_INVALID_REF) {
+ gnttab_end_foreign_access(persistent_gnt->gref,
+ 0, 0UL);
+ rinfo->persistent_gnts_c--;
+ }
+ if (info->feature_persistent)
+ __free_page(pfn_to_page(persistent_gnt->pfn));
+ kfree(persistent_gnt);
+ }
+ }
+ BUG_ON(rinfo->persistent_gnts_c != 0);
+
for (i = 0; i < BLK_RING_SIZE(info); i++) {
/*
* Clear persistent grants present in requests already
@@ -1075,37 +1084,17 @@ free_shadow:
static void blkif_free(struct blkfront_info *info, int suspend)
{
- struct grant *persistent_gnt, *n;
int i;
/* Prevent new requests being issued until we fix things up. */
- spin_lock_irq(&info->dev_lock);
info->connected = suspend ?
BLKIF_STATE_SUSPENDED : BLKIF_STATE_DISCONNECTED;
/* No more blkif_request(). */
if (info->rq)
blk_mq_stop_hw_queues(info->rq);
- /* Remove all persistent grants */
- if (!list_empty(&info->grants)) {
- list_for_each_entry_safe(persistent_gnt, n,
- &info->grants, node) {
- list_del(&persistent_gnt->node);
- if (persistent_gnt->gref != GRANT_INVALID_REF) {
- gnttab_end_foreign_access(persistent_gnt->gref,
- 0, 0UL);
- info->persistent_gnts_c--;
- }
- if (info->feature_persistent)
- __free_page(pfn_to_page(persistent_gnt->pfn));
- kfree(persistent_gnt);
- }
- }
- BUG_ON(info->persistent_gnts_c != 0);
-
for (i = 0; i < info->nr_rings; i++)
blkif_free_ring(&info->rinfo[i]);
- spin_unlock_irq(&info->dev_lock);
}
static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *rinfo,
@@ -1117,7 +1106,6 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
void *shared_data;
int nseg;
struct blkfront_info *info = rinfo->dev_info;
- unsigned long flags;
nseg = s->req.operation == BLKIF_OP_INDIRECT ?
s->req.u.indirect.nr_segments : s->req.u.rw.nr_segments;
@@ -1148,10 +1136,8 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
if (!info->feature_persistent)
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
s->grants_used[i]->gref);
- spin_lock_irqsave(&info->dev_lock, flags);
- list_add(&s->grants_used[i]->node, &info->grants);
- info->persistent_gnts_c++;
- spin_unlock_irqrestore(&info->dev_lock, flags);
+ list_add(&s->grants_used[i]->node, &rinfo->grants);
+ rinfo->persistent_gnts_c++;
} else {
/*
* If the grant is not mapped by the backend we end the
@@ -1161,9 +1147,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
*/
gnttab_end_foreign_access(s->grants_used[i]->gref, 0, 0UL);
s->grants_used[i]->gref = GRANT_INVALID_REF;
- spin_lock_irqsave(&info->dev_lock, flags);
- list_add_tail(&s->grants_used[i]->node, &info->grants);
- spin_unlock_irqrestore(&info->dev_lock, flags);
+ list_add_tail(&s->grants_used[i]->node, &rinfo->grants);
}
}
if (s->req.operation == BLKIF_OP_INDIRECT) {
@@ -1172,10 +1156,8 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
if (!info->feature_persistent)
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
s->indirect_grants[i]->gref);
- spin_lock_irqsave(&info->dev_lock, flags);
- list_add(&s->indirect_grants[i]->node, &info->grants);
- info->persistent_gnts_c++;
- spin_unlock_irqrestore(&info->dev_lock, flags);
+ list_add(&s->indirect_grants[i]->node, &rinfo->grants);
+ rinfo->persistent_gnts_c++;
} else {
struct page *indirect_page;
@@ -1184,14 +1166,12 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_ring_info *ri
* Add the used indirect page back to the list of
* available pages for indirect grefs.
*/
- spin_lock_irqsave(&info->dev_lock, flags);
if (!info->feature_persistent) {
indirect_page = pfn_to_page(s->indirect_grants[i]->pfn);
list_add(&indirect_page->lru, &rinfo->indirect_pages);
}
s->indirect_grants[i]->gref = GRANT_INVALID_REF;
- list_add_tail(&s->indirect_grants[i]->node, &info->grants);
- spin_unlock_irqrestore(&info->dev_lock, flags);
+ list_add_tail(&s->indirect_grants[i]->node, &rinfo->grants);
}
}
}
@@ -1587,10 +1567,8 @@ static int blkfront_probe(struct xenbus_device *dev,
}
mutex_init(&info->mutex);
- spin_lock_init(&info->dev_lock);
info->xbdev = dev;
info->vdevice = vdevice;
- INIT_LIST_HEAD(&info->grants);
info->connected = BLKIF_STATE_DISCONNECTED;
/* Check if backend supports multiple queues */
@@ -1616,6 +1594,7 @@ static int blkfront_probe(struct xenbus_device *dev,
struct blkfront_ring_info *rinfo;
rinfo = &info->rinfo[r_index];
+ INIT_LIST_HEAD(&rinfo->grants);
INIT_LIST_HEAD(&rinfo->indirect_pages);
rinfo->dev_info = info;
INIT_WORK(&rinfo->work, blkif_restart_queue);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue
2015-11-02 4:21 ` [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue Bob Liu
@ 2015-11-05 2:39 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 2:39 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On Mon, Nov 02, 2015 at 12:21:45PM +0800, Bob Liu wrote:
> Make persistent grants per-queue/ring instead of per-device, so that we can
> drop the 'dev_lock' and get better scalability.
And what is the performance value for this? How much better
scalability do you get with this?
.. snip..
> @@ -1010,6 +1002,23 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
> }
> }
>
> + /* Remove all persistent grants */
Missing full stop.
> + if (!list_empty(&rinfo->grants)) {
> + list_for_each_entry_safe(persistent_gnt, n,
> + &rinfo->grants, node) {
> + list_del(&persistent_gnt->node);
> + if (persistent_gnt->gref != GRANT_INVALID_REF) {
> + gnttab_end_foreign_access(persistent_gnt->gref,
> + 0, 0UL);
> + rinfo->persistent_gnts_c--;
> + }
> + if (info->feature_persistent)
> + __free_page(pfn_to_page(persistent_gnt->pfn));
> + kfree(persistent_gnt);
> + }
> + }
> + BUG_ON(rinfo->persistent_gnts_c != 0);
> +
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (8 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 09/10] xen/blkfront: make persistent grants per-queue Bob Liu
@ 2015-11-02 4:21 ` Bob Liu
2015-11-05 2:43 ` Konrad Rzeszutek Wilk
2015-11-02 11:19 ` [Xen-devel] [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Julien Grall
10 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-02 4:21 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, roger.pau, konrad.wilk, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel, Bob Liu
Make pool of persistent grants and free pages per-queue/ring instead of
per-device to get better scalability.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
drivers/block/xen-blkback/blkback.c | 212 +++++++++++++++++-------------------
drivers/block/xen-blkback/common.h | 32 +++---
drivers/block/xen-blkback/xenbus.c | 21 ++--
3 files changed, 124 insertions(+), 141 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 107cc4a..28cbdae 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -118,60 +118,60 @@ module_param(log_stats, int, 0644);
/* Number of free pages to remove on each call to gnttab_free_pages */
#define NUM_BATCH_FREE_PAGES 10
-static inline int get_free_page(struct xen_blkif *blkif, struct page **page)
+static inline int get_free_page(struct xen_blkif_ring *ring, struct page **page)
{
unsigned long flags;
- spin_lock_irqsave(&blkif->free_pages_lock, flags);
- if (list_empty(&blkif->free_pages)) {
- BUG_ON(blkif->free_pages_num != 0);
- spin_unlock_irqrestore(&blkif->free_pages_lock, flags);
+ spin_lock_irqsave(&ring->free_pages_lock, flags);
+ if (list_empty(&ring->free_pages)) {
+ BUG_ON(ring->free_pages_num != 0);
+ spin_unlock_irqrestore(&ring->free_pages_lock, flags);
return gnttab_alloc_pages(1, page);
}
- BUG_ON(blkif->free_pages_num == 0);
- page[0] = list_first_entry(&blkif->free_pages, struct page, lru);
+ BUG_ON(ring->free_pages_num == 0);
+ page[0] = list_first_entry(&ring->free_pages, struct page, lru);
list_del(&page[0]->lru);
- blkif->free_pages_num--;
- spin_unlock_irqrestore(&blkif->free_pages_lock, flags);
+ ring->free_pages_num--;
+ spin_unlock_irqrestore(&ring->free_pages_lock, flags);
return 0;
}
-static inline void put_free_pages(struct xen_blkif *blkif, struct page **page,
+static inline void put_free_pages(struct xen_blkif_ring *ring, struct page **page,
int num)
{
unsigned long flags;
int i;
- spin_lock_irqsave(&blkif->free_pages_lock, flags);
+ spin_lock_irqsave(&ring->free_pages_lock, flags);
for (i = 0; i < num; i++)
- list_add(&page[i]->lru, &blkif->free_pages);
- blkif->free_pages_num += num;
- spin_unlock_irqrestore(&blkif->free_pages_lock, flags);
+ list_add(&page[i]->lru, &ring->free_pages);
+ ring->free_pages_num += num;
+ spin_unlock_irqrestore(&ring->free_pages_lock, flags);
}
-static inline void shrink_free_pagepool(struct xen_blkif *blkif, int num)
+static inline void shrink_free_pagepool(struct xen_blkif_ring *ring, int num)
{
/* Remove requested pages in batches of NUM_BATCH_FREE_PAGES */
struct page *page[NUM_BATCH_FREE_PAGES];
unsigned int num_pages = 0;
unsigned long flags;
- spin_lock_irqsave(&blkif->free_pages_lock, flags);
- while (blkif->free_pages_num > num) {
- BUG_ON(list_empty(&blkif->free_pages));
- page[num_pages] = list_first_entry(&blkif->free_pages,
+ spin_lock_irqsave(&ring->free_pages_lock, flags);
+ while (ring->free_pages_num > num) {
+ BUG_ON(list_empty(&ring->free_pages));
+ page[num_pages] = list_first_entry(&ring->free_pages,
struct page, lru);
list_del(&page[num_pages]->lru);
- blkif->free_pages_num--;
+ ring->free_pages_num--;
if (++num_pages == NUM_BATCH_FREE_PAGES) {
- spin_unlock_irqrestore(&blkif->free_pages_lock, flags);
+ spin_unlock_irqrestore(&ring->free_pages_lock, flags);
gnttab_free_pages(num_pages, page);
- spin_lock_irqsave(&blkif->free_pages_lock, flags);
+ spin_lock_irqsave(&ring->free_pages_lock, flags);
num_pages = 0;
}
}
- spin_unlock_irqrestore(&blkif->free_pages_lock, flags);
+ spin_unlock_irqrestore(&ring->free_pages_lock, flags);
if (num_pages != 0)
gnttab_free_pages(num_pages, page);
}
@@ -194,22 +194,29 @@ static void make_response(struct xen_blkif_ring *ring, u64 id,
/*
- * pers_gnts_lock must be used around all the persistent grant helpers
- * because blkback may use multi-thread/queue for each backend.
+ * We don't need locking around the persistent grant helpers
+ * because blkback uses a single-thread for each backed, so we
+ * can be sure that this functions will never be called recursively.
+ *
+ * The only exception to that is put_persistent_grant, that can be called
+ * from interrupt context (by xen_blkbk_unmap), so we have to use atomic
+ * bit operations to modify the flags of a persistent grant and to count
+ * the number of used grants.
*/
-static int add_persistent_gnt(struct xen_blkif *blkif,
+static int add_persistent_gnt(struct xen_blkif_ring *ring,
struct persistent_gnt *persistent_gnt)
{
struct rb_node **new = NULL, *parent = NULL;
struct persistent_gnt *this;
+ struct xen_blkif *blkif = ring->blkif;
- if (blkif->persistent_gnt_c >= xen_blkif_max_pgrants) {
+ if (ring->persistent_gnt_c >= xen_blkif_max_pgrants) {
if (!blkif->vbd.overflow_max_grants)
blkif->vbd.overflow_max_grants = 1;
return -EBUSY;
}
/* Figure out where to put new node */
- new = &blkif->persistent_gnts.rb_node;
+ new = &ring->persistent_gnts.rb_node;
while (*new) {
this = container_of(*new, struct persistent_gnt, node);
@@ -228,19 +235,19 @@ static int add_persistent_gnt(struct xen_blkif *blkif,
set_bit(PERSISTENT_GNT_ACTIVE, persistent_gnt->flags);
/* Add new node and rebalance tree. */
rb_link_node(&(persistent_gnt->node), parent, new);
- rb_insert_color(&(persistent_gnt->node), &blkif->persistent_gnts);
- blkif->persistent_gnt_c++;
- atomic_inc(&blkif->persistent_gnt_in_use);
+ rb_insert_color(&(persistent_gnt->node), &ring->persistent_gnts);
+ ring->persistent_gnt_c++;
+ atomic_inc(&ring->persistent_gnt_in_use);
return 0;
}
-static struct persistent_gnt *get_persistent_gnt(struct xen_blkif *blkif,
+static struct persistent_gnt *get_persistent_gnt(struct xen_blkif_ring *ring,
grant_ref_t gref)
{
struct persistent_gnt *data;
struct rb_node *node = NULL;
- node = blkif->persistent_gnts.rb_node;
+ node = ring->persistent_gnts.rb_node;
while (node) {
data = container_of(node, struct persistent_gnt, node);
@@ -254,24 +261,24 @@ static struct persistent_gnt *get_persistent_gnt(struct xen_blkif *blkif,
return NULL;
}
set_bit(PERSISTENT_GNT_ACTIVE, data->flags);
- atomic_inc(&blkif->persistent_gnt_in_use);
+ atomic_inc(&ring->persistent_gnt_in_use);
return data;
}
}
return NULL;
}
-static void put_persistent_gnt(struct xen_blkif *blkif,
+static void put_persistent_gnt(struct xen_blkif_ring *ring,
struct persistent_gnt *persistent_gnt)
{
if(!test_bit(PERSISTENT_GNT_ACTIVE, persistent_gnt->flags))
pr_alert_ratelimited("freeing a grant already unused\n");
set_bit(PERSISTENT_GNT_WAS_ACTIVE, persistent_gnt->flags);
clear_bit(PERSISTENT_GNT_ACTIVE, persistent_gnt->flags);
- atomic_dec(&blkif->persistent_gnt_in_use);
+ atomic_dec(&ring->persistent_gnt_in_use);
}
-static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
+static void free_persistent_gnts(struct xen_blkif_ring *ring, struct rb_root *root,
unsigned int num)
{
struct gnttab_unmap_grant_ref unmap[BLKIF_MAX_SEGMENTS_PER_REQUEST];
@@ -302,7 +309,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
unmap_data.count = segs_to_unmap;
BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
- put_free_pages(blkif, pages, segs_to_unmap);
+ put_free_pages(ring, pages, segs_to_unmap);
segs_to_unmap = 0;
}
@@ -319,17 +326,15 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
struct page *pages[BLKIF_MAX_SEGMENTS_PER_REQUEST];
struct persistent_gnt *persistent_gnt;
int segs_to_unmap = 0;
- struct xen_blkif *blkif = container_of(work, typeof(*blkif), persistent_purge_work);
+ struct xen_blkif_ring *ring = container_of(work, typeof(*ring), persistent_purge_work);
struct gntab_unmap_queue_data unmap_data;
- unsigned long flags;
unmap_data.pages = pages;
unmap_data.unmap_ops = unmap;
unmap_data.kunmap_ops = NULL;
- spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
- while(!list_empty(&blkif->persistent_purge_list)) {
- persistent_gnt = list_first_entry(&blkif->persistent_purge_list,
+ while(!list_empty(&ring->persistent_purge_list)) {
+ persistent_gnt = list_first_entry(&ring->persistent_purge_list,
struct persistent_gnt,
remove_node);
list_del(&persistent_gnt->remove_node);
@@ -344,45 +349,42 @@ void xen_blkbk_unmap_purged_grants(struct work_struct *work)
if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
unmap_data.count = segs_to_unmap;
BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
- put_free_pages(blkif, pages, segs_to_unmap);
+ put_free_pages(ring, pages, segs_to_unmap);
segs_to_unmap = 0;
}
kfree(persistent_gnt);
}
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
if (segs_to_unmap > 0) {
unmap_data.count = segs_to_unmap;
BUG_ON(gnttab_unmap_refs_sync(&unmap_data));
- put_free_pages(blkif, pages, segs_to_unmap);
+ put_free_pages(ring, pages, segs_to_unmap);
}
}
-static void purge_persistent_gnt(struct xen_blkif *blkif)
+static void purge_persistent_gnt(struct xen_blkif_ring *ring)
{
struct persistent_gnt *persistent_gnt;
struct rb_node *n;
unsigned int num_clean, total;
bool scan_used = false, clean_used = false;
struct rb_root *root;
- unsigned long flags;
- spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
- if (blkif->persistent_gnt_c < xen_blkif_max_pgrants ||
- (blkif->persistent_gnt_c == xen_blkif_max_pgrants &&
- !blkif->vbd.overflow_max_grants)) {
+ if (ring->persistent_gnt_c < xen_blkif_max_pgrants ||
+ (ring->persistent_gnt_c == xen_blkif_max_pgrants &&
+ !ring->blkif->vbd.overflow_max_grants)) {
goto out;
}
- if (work_busy(&blkif->persistent_purge_work)) {
+ if (work_busy(&ring->persistent_purge_work)) {
pr_alert_ratelimited("Scheduled work from previous purge is still busy, cannot purge list\n");
goto out;
}
num_clean = (xen_blkif_max_pgrants / 100) * LRU_PERCENT_CLEAN;
- num_clean = blkif->persistent_gnt_c - xen_blkif_max_pgrants + num_clean;
- num_clean = min(blkif->persistent_gnt_c, num_clean);
+ num_clean = ring->persistent_gnt_c - xen_blkif_max_pgrants + num_clean;
+ num_clean = min(ring->persistent_gnt_c, num_clean);
if ((num_clean == 0) ||
- (num_clean > (blkif->persistent_gnt_c - atomic_read(&blkif->persistent_gnt_in_use))))
+ (num_clean > (ring->persistent_gnt_c - atomic_read(&ring->persistent_gnt_in_use))))
goto out;
/*
@@ -398,8 +400,8 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)
pr_debug("Going to purge %u persistent grants\n", num_clean);
- BUG_ON(!list_empty(&blkif->persistent_purge_list));
- root = &blkif->persistent_gnts;
+ BUG_ON(!list_empty(&ring->persistent_purge_list));
+ root = &ring->persistent_gnts;
purge_list:
foreach_grant_safe(persistent_gnt, n, root, node) {
BUG_ON(persistent_gnt->handle ==
@@ -418,7 +420,7 @@ purge_list:
rb_erase(&persistent_gnt->node, root);
list_add(&persistent_gnt->remove_node,
- &blkif->persistent_purge_list);
+ &ring->persistent_purge_list);
if (--num_clean == 0)
goto finished;
}
@@ -439,18 +441,14 @@ finished:
goto purge_list;
}
- blkif->persistent_gnt_c -= (total - num_clean);
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
- blkif->vbd.overflow_max_grants = 0;
+ ring->persistent_gnt_c -= (total - num_clean);
+ ring->blkif->vbd.overflow_max_grants = 0;
/* We can defer this work */
- schedule_work(&blkif->persistent_purge_work);
+ schedule_work(&ring->persistent_purge_work);
pr_debug("Purged %u/%u\n", (total - num_clean), total);
- return;
out:
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
-
return;
}
@@ -582,20 +580,20 @@ irqreturn_t xen_blkif_be_int(int irq, void *dev_id)
* SCHEDULER FUNCTIONS
*/
-static void print_stats(struct xen_blkif *blkif)
+static void print_stats(struct xen_blkif_ring *ring)
{
pr_info("(%s): oo %3llu | rd %4llu | wr %4llu | f %4llu"
" | ds %4llu | pg: %4u/%4d\n",
- current->comm, blkif->st_oo_req,
- blkif->st_rd_req, blkif->st_wr_req,
- blkif->st_f_req, blkif->st_ds_req,
- blkif->persistent_gnt_c,
+ current->comm, ring->st_oo_req,
+ ring->st_rd_req, ring->st_wr_req,
+ ring->st_f_req, ring->st_ds_req,
+ ring->persistent_gnt_c,
xen_blkif_max_pgrants);
- blkif->st_print = jiffies + msecs_to_jiffies(10 * 1000);
- blkif->st_rd_req = 0;
- blkif->st_wr_req = 0;
- blkif->st_oo_req = 0;
- blkif->st_ds_req = 0;
+ ring->st_print = jiffies + msecs_to_jiffies(10 * 1000);
+ ring->st_rd_req = 0;
+ ring->st_wr_req = 0;
+ ring->st_oo_req = 0;
+ ring->st_ds_req = 0;
}
int xen_blkif_schedule(void *arg)
@@ -642,23 +640,23 @@ int xen_blkif_schedule(void *arg)
purge_gnt_list:
if (blkif->vbd.feature_gnt_persistent &&
- time_after(jiffies, blkif->next_lru)) {
- purge_persistent_gnt(blkif);
- blkif->next_lru = jiffies + msecs_to_jiffies(LRU_INTERVAL);
+ time_after(jiffies, ring->next_lru)) {
+ purge_persistent_gnt(ring);
+ ring->next_lru = jiffies + msecs_to_jiffies(LRU_INTERVAL);
}
/* Shrink if we have more than xen_blkif_max_buffer_pages */
- shrink_free_pagepool(blkif, xen_blkif_max_buffer_pages);
+ shrink_free_pagepool(ring, xen_blkif_max_buffer_pages);
- if (log_stats && time_after(jiffies, blkif->st_print))
- print_stats(blkif);
+ if (log_stats && time_after(jiffies, ring->st_print))
+ print_stats(ring);
}
/* Drain pending purge work */
- flush_work(&blkif->persistent_purge_work);
+ flush_work(&ring->persistent_purge_work);
if (log_stats)
- print_stats(blkif);
+ print_stats(ring);
ring->xenblkd = NULL;
xen_blkif_put(blkif);
@@ -671,21 +669,16 @@ purge_gnt_list:
*/
void xen_blkbk_free_caches(struct xen_blkif_ring *ring)
{
- struct xen_blkif *blkif = ring->blkif;
- unsigned long flags;
-
/* Free all persistent grant pages */
- spin_lock_irqsave(&blkif->pers_gnts_lock, flags);
- if (!RB_EMPTY_ROOT(&blkif->persistent_gnts))
- free_persistent_gnts(blkif, &blkif->persistent_gnts,
- blkif->persistent_gnt_c);
+ if (!RB_EMPTY_ROOT(&ring->persistent_gnts))
+ free_persistent_gnts(ring, &ring->persistent_gnts,
+ ring->persistent_gnt_c);
- BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
- blkif->persistent_gnt_c = 0;
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, flags);
+ BUG_ON(!RB_EMPTY_ROOT(&ring->persistent_gnts));
+ ring->persistent_gnt_c = 0;
/* Since we are shutting down remove all pages from the buffer */
- shrink_free_pagepool(blkif, 0 /* All */);
+ shrink_free_pagepool(ring, 0 /* All */);
}
static unsigned int xen_blkbk_unmap_prepare(
@@ -696,13 +689,10 @@ static unsigned int xen_blkbk_unmap_prepare(
struct page **unmap_pages)
{
unsigned int i, invcount = 0;
- unsigned long flags;
for (i = 0; i < num; i++) {
if (pages[i]->persistent_gnt != NULL) {
- spin_lock_irqsave(&ring->blkif->pers_gnts_lock, flags);
- put_persistent_gnt(ring->blkif, pages[i]->persistent_gnt);
- spin_unlock_irqrestore(&ring->blkif->pers_gnts_lock, flags);
+ put_persistent_gnt(ring, pages[i]->persistent_gnt);
continue;
}
if (pages[i]->handle == BLKBACK_INVALID_HANDLE)
@@ -727,7 +717,7 @@ static void xen_blkbk_unmap_and_respond_callback(int result, struct gntab_unmap_
but is this the best way to deal with this? */
BUG_ON(result);
- put_free_pages(blkif, data->pages, data->count);
+ put_free_pages(ring, data->pages, data->count);
make_response(ring, pending_req->id,
pending_req->operation, pending_req->status);
free_req(ring, pending_req);
@@ -794,7 +784,7 @@ static void xen_blkbk_unmap(struct xen_blkif_ring *ring,
if (invcount) {
ret = gnttab_unmap_refs(unmap, NULL, unmap_pages, invcount);
BUG_ON(ret);
- put_free_pages(ring->blkif, unmap_pages, invcount);
+ put_free_pages(ring, unmap_pages, invcount);
}
pages += batch;
num -= batch;
@@ -815,7 +805,6 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
int last_map = 0, map_until = 0;
int use_persistent_gnts;
struct xen_blkif *blkif = ring->blkif;
- unsigned long irq_flags;
use_persistent_gnts = (blkif->vbd.feature_gnt_persistent);
@@ -829,11 +818,9 @@ again:
uint32_t flags;
if (use_persistent_gnts) {
- spin_lock_irqsave(&blkif->pers_gnts_lock, irq_flags);
persistent_gnt = get_persistent_gnt(
- blkif,
+ ring,
pages[i]->gref);
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
}
if (persistent_gnt) {
@@ -844,7 +831,7 @@ again:
pages[i]->page = persistent_gnt->page;
pages[i]->persistent_gnt = persistent_gnt;
} else {
- if (get_free_page(blkif, &pages[i]->page))
+ if (get_free_page(ring, &pages[i]->page))
goto out_of_memory;
addr = vaddr(pages[i]->page);
pages_to_gnt[segs_to_map] = pages[i]->page;
@@ -877,7 +864,7 @@ again:
BUG_ON(new_map_idx >= segs_to_map);
if (unlikely(map[new_map_idx].status != 0)) {
pr_debug("invalid buffer -- could not remap it\n");
- put_free_pages(blkif, &pages[seg_idx]->page, 1);
+ put_free_pages(ring, &pages[seg_idx]->page, 1);
pages[seg_idx]->handle = BLKBACK_INVALID_HANDLE;
ret |= 1;
goto next;
@@ -887,7 +874,7 @@ again:
continue;
}
if (use_persistent_gnts &&
- blkif->persistent_gnt_c < xen_blkif_max_pgrants) {
+ ring->persistent_gnt_c < xen_blkif_max_pgrants) {
/*
* We are using persistent grants, the grant is
* not mapped but we might have room for it.
@@ -905,19 +892,16 @@ again:
persistent_gnt->gnt = map[new_map_idx].ref;
persistent_gnt->handle = map[new_map_idx].handle;
persistent_gnt->page = pages[seg_idx]->page;
- spin_lock_irqsave(&blkif->pers_gnts_lock, irq_flags);
- if (add_persistent_gnt(blkif,
+ if (add_persistent_gnt(ring,
persistent_gnt)) {
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
kfree(persistent_gnt);
persistent_gnt = NULL;
goto next;
}
pages[seg_idx]->persistent_gnt = persistent_gnt;
pr_debug("grant %u added to the tree of persistent grants, using %u/%u\n",
- persistent_gnt->gnt, blkif->persistent_gnt_c,
+ persistent_gnt->gnt, ring->persistent_gnt_c,
xen_blkif_max_pgrants);
- spin_unlock_irqrestore(&blkif->pers_gnts_lock, irq_flags);
goto next;
}
if (use_persistent_gnts && !blkif->vbd.overflow_max_grants) {
@@ -941,7 +925,7 @@ next:
out_of_memory:
pr_alert("%s: out of memory\n", __func__);
- put_free_pages(blkif, pages_to_gnt, segs_to_map);
+ put_free_pages(ring, pages_to_gnt, segs_to_map);
return -ENOMEM;
}
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index fb28b91..5f9d0ee 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -282,6 +282,22 @@ struct xen_blkif_ring {
spinlock_t pending_free_lock;
wait_queue_head_t pending_free_wq;
+ /* buffer of free pages to map grant refs */
+ spinlock_t free_pages_lock;
+ int free_pages_num;
+ struct list_head free_pages;
+
+ /* tree to store persistent grants */
+ spinlock_t pers_gnts_lock;
+ struct rb_root persistent_gnts;
+ unsigned int persistent_gnt_c;
+ atomic_t persistent_gnt_in_use;
+ unsigned long next_lru;
+
+ /* used by the kworker that offload work from the persistent purge */
+ struct list_head persistent_purge_list;
+ struct work_struct persistent_purge_work;
+
/* statistics */
unsigned long st_print;
unsigned long long st_rd_req;
@@ -313,22 +329,6 @@ struct xen_blkif {
struct completion drain_complete;
atomic_t drain;
- /* tree to store persistent grants */
- spinlock_t pers_gnts_lock;
- struct rb_root persistent_gnts;
- unsigned int persistent_gnt_c;
- atomic_t persistent_gnt_in_use;
- unsigned long next_lru;
-
- /* used by the kworker that offload work from the persistent purge */
- struct list_head persistent_purge_list;
- struct work_struct persistent_purge_work;
-
- /* buffer of free pages to map grant refs */
- spinlock_t free_pages_lock;
- int free_pages_num;
- struct list_head free_pages;
-
/* statistics */
unsigned long st_print;
unsigned long long st_rd_req;
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index cafbadd..dc5ce88 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -151,6 +151,10 @@ static int xen_blkif_alloc_rings(struct xen_blkif *blkif)
spin_lock_init(&ring->blk_ring_lock);
init_waitqueue_head(&ring->wq);
INIT_LIST_HEAD(&ring->pending_free);
+ INIT_LIST_HEAD(&ring->persistent_purge_list);
+ INIT_WORK(&ring->persistent_purge_work, xen_blkbk_unmap_purged_grants);
+ spin_lock_init(&ring->free_pages_lock);
+ INIT_LIST_HEAD(&ring->free_pages);
spin_lock_init(&ring->pending_free_lock);
init_waitqueue_head(&ring->pending_free_wq);
@@ -176,10 +180,6 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
atomic_set(&blkif->refcnt, 1);
init_completion(&blkif->drain_complete);
INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
- spin_lock_init(&blkif->free_pages_lock);
- INIT_LIST_HEAD(&blkif->free_pages);
- INIT_LIST_HEAD(&blkif->persistent_purge_list);
- INIT_WORK(&blkif->persistent_purge_work, xen_blkbk_unmap_purged_grants);
return blkif;
}
@@ -287,6 +287,12 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
i++;
}
+ BUG_ON(atomic_read(&ring->persistent_gnt_in_use) != 0);
+ BUG_ON(!list_empty(&ring->persistent_purge_list));
+ BUG_ON(!RB_EMPTY_ROOT(&ring->persistent_gnts));
+ BUG_ON(!list_empty(&ring->free_pages));
+ BUG_ON(ring->free_pages_num != 0);
+ BUG_ON(ring->persistent_gnt_c != 0);
WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
}
blkif->nr_ring_pages = 0;
@@ -301,13 +307,6 @@ static void xen_blkif_free(struct xen_blkif *blkif)
xen_vbd_free(&blkif->vbd);
/* Make sure everything is drained before shutting down */
- BUG_ON(blkif->persistent_gnt_c != 0);
- BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
- BUG_ON(blkif->free_pages_num != 0);
- BUG_ON(!list_empty(&blkif->persistent_purge_list));
- BUG_ON(!list_empty(&blkif->free_pages));
- BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
-
kfree(blkif->rings);
kmem_cache_free(xen_blkif_cachep, blkif);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue
2015-11-02 4:21 ` [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue Bob Liu
@ 2015-11-05 2:43 ` Konrad Rzeszutek Wilk
2015-11-05 2:46 ` Bob Liu
0 siblings, 1 reply; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 2:43 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On Mon, Nov 02, 2015 at 12:21:46PM +0800, Bob Liu wrote:
> Make pool of persistent grants and free pages per-queue/ring instead of
> per-device to get better scalability.
How much better scalability do we get?
.. snip ..
>
>
> /*
> - * pers_gnts_lock must be used around all the persistent grant helpers
> - * because blkback may use multi-thread/queue for each backend.
> + * We don't need locking around the persistent grant helpers
> + * because blkback uses a single-thread for each backed, so we
s/backed/backend/
> + * can be sure that this functions will never be called recursively.
> + *
> + * The only exception to that is put_persistent_grant, that can be called
> + * from interrupt context (by xen_blkbk_unmap), so we have to use atomic
> + * bit operations to modify the flags of a persistent grant and to count
> + * the number of used grants.
> */
..snip..
> --- a/drivers/block/xen-blkback/common.h
> +++ b/drivers/block/xen-blkback/common.h
> @@ -282,6 +282,22 @@ struct xen_blkif_ring {
> spinlock_t pending_free_lock;
> wait_queue_head_t pending_free_wq;
>
> + /* buffer of free pages to map grant refs */
Full stop.
> + spinlock_t free_pages_lock;
> + int free_pages_num;
> + struct list_head free_pages;
> +
> + /* tree to store persistent grants */
Full stop.
> + spinlock_t pers_gnts_lock;
> + struct rb_root persistent_gnts;
> + unsigned int persistent_gnt_c;
> + atomic_t persistent_gnt_in_use;
> + unsigned long next_lru;
> +
> + /* used by the kworker that offload work from the persistent purge */
Full stop.
> + struct list_head persistent_purge_list;
> + struct work_struct persistent_purge_work;
> +
> /* statistics */
> unsigned long st_print;
> unsigned long long st_rd_req;
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue
2015-11-05 2:43 ` Konrad Rzeszutek Wilk
@ 2015-11-05 2:46 ` Bob Liu
2015-11-05 19:50 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 27+ messages in thread
From: Bob Liu @ 2015-11-05 2:46 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On 11/05/2015 10:43 AM, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 02, 2015 at 12:21:46PM +0800, Bob Liu wrote:
>> Make pool of persistent grants and free pages per-queue/ring instead of
>> per-device to get better scalability.
>
> How much better scalability do we get?
>
Which already showed in [00/10], I paste them here:
domU(orig) 4 queues 8 queues 16 queues
iops: 690k 1024k(+30%) 800k 750k
After patch 9 and 10:
domU(orig) 4 queues 8 queues 16 queues
iops: 690k 1600k(+100%) 1450k 1320k
Chart: https://www.dropbox.com/s/agrcy2pbzbsvmwv/iops.png?dl=0
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue
2015-11-05 2:46 ` Bob Liu
@ 2015-11-05 19:50 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-11-05 19:50 UTC (permalink / raw)
To: Bob Liu
Cc: xen-devel, linux-kernel, roger.pau, felipe.franciosi, axboe,
avanzini.arianna, rafal.mielniczuk, jonathan.davies,
david.vrabel
On Thu, Nov 05, 2015 at 10:46:05AM +0800, Bob Liu wrote:
>
> On 11/05/2015 10:43 AM, Konrad Rzeszutek Wilk wrote:
> > On Mon, Nov 02, 2015 at 12:21:46PM +0800, Bob Liu wrote:
> >> Make pool of persistent grants and free pages per-queue/ring instead of
> >> per-device to get better scalability.
> >
> > How much better scalability do we get?
> >
>
> Which already showed in [00/10], I paste them here:
>
> domU(orig) 4 queues 8 queues 16 queues
> iops: 690k 1024k(+30%) 800k 750k
>
>
> After patch 9 and 10:
> domU(orig) 4 queues 8 queues 16 queues
> iops: 690k 1600k(+100%) 1450k 1320k
And if you split it? As in only patch 9?
If you could - please mention in the description that with
4 queues with this patch you get 100% increase in IOPS.
>
> Chart: https://www.dropbox.com/s/agrcy2pbzbsvmwv/iops.png?dl=0
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Xen-devel] [PATCH v4 00/10] xen-block: multi hardware-queues/rings support
2015-11-02 4:21 [PATCH v4 00/10] xen-block: multi hardware-queues/rings support Bob Liu
` (9 preceding siblings ...)
2015-11-02 4:21 ` [PATCH v4 10/10] xen/blkback: make pool of persistent grants and free pages per-queue Bob Liu
@ 2015-11-02 11:19 ` Julien Grall
10 siblings, 0 replies; 27+ messages in thread
From: Julien Grall @ 2015-11-02 11:19 UTC (permalink / raw)
To: Bob Liu, xen-devel
Cc: jonathan.davies, felipe.franciosi, rafal.mielniczuk,
linux-kernel, axboe, david.vrabel, avanzini.arianna, roger.pau
Hi Bob,
On 02/11/15 04:21, Bob Liu wrote:
> ---
> v4:
> * Rebase to v4.3-rc7
xentip/for-linus-4.4 [1] contains a lot of change in xen-blkfront,
mainly to add support of 64KB page granularity.
I would advise you to rebase your series know as some part of the code
may have changed.
Regards,
[1]
https://git.kernel.org/cgit/linux/kernel/git/xen/tip.git/log/?h=for-linus-4.4
--
Julien Grall
^ permalink raw reply [flat|nested] 27+ messages in thread
