OK, this is (ab)uses some of Paolo's patches. The first 7 are candidates for this merge window (maybe), the rest I'm not so sure about. Thanks, Rusty. Paolo Bonzini (3): scatterlist: introduce sg_unmark_end virtio-blk: reorganize virtblk_add_req virtio-blk: use virtqueue_add_sgs on req path Rusty Russell (13): virtio_ring: virtqueue_add_sgs, to add multiple sgs. virtio-blk: use virtqueue_start_buf on bio path virtio_blk: remove nents member. virtio_ring: don't count elements twice for add_buf path. virtio_ring: virtqueue_add_outbuf / virtqueue_add_inbuf. virtio_net: use simplified virtqueue accessors. virtio_net: use virtqueue_add_sgs[] for command buffers. virtio_rng: use simplified virtqueue accessors. virtio_console: use simplified virtqueue accessors. caif_virtio: use simplified virtqueue accessors. virtio_rpmsg_bus: use simplified virtqueue accessors. virtio_balloon: use simplified virtqueue accessors. 9p/trans_virtio.c: use virtio_add_sgs[] block/blk-integrity.c | 2 +- block/blk-merge.c | 2 +- drivers/block/virtio_blk.c | 146 +++++++++----------- drivers/char/hw_random/virtio-rng.c | 2 +- drivers/char/virtio_console.c | 6 +- drivers/net/caif/caif_virtio.c | 3 +- drivers/net/virtio_net.c | 61 ++++----- drivers/rpmsg/virtio_rpmsg_bus.c | 8 +- drivers/virtio/virtio_balloon.c | 6 +- drivers/virtio/virtio_ring.c | 253 ++++++++++++++++++++++++++--------- include/linux/scatterlist.h | 16 +++ include/linux/virtio.h | 17 +++ net/9p/trans_virtio.c | 48 +++++-- 13 files changed, 371 insertions(+), 199 deletions(-) -- 1.7.10.4
From: Paolo Bonzini <pbonzini at redhat.com>
This is useful in places that recycle the same scatterlist multiple
times, and do not want to incur the cost of sg_init_table every
time in hot paths.
Acked-by: Jens Axboe <axboe at kernel.dk>
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
block/blk-integrity.c | 2 +-
block/blk-merge.c | 2 +-
include/linux/scatterlist.h | 16 ++++++++++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index da2a818..0f5004f 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -110,7 +110,7 @@ new_segment:
if (!sg)
sg = sglist;
else {
- sg->page_link &= ~0x02;
+ sg_unmark_end(sg);
sg = sg_next(sg);
}
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 936a110..5f24482 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -143,7 +143,7 @@ new_segment:
* termination bit to avoid doing a full
* sg_init_table() in drivers for each command.
*/
- (*sg)->page_link &= ~0x02;
+ sg_unmark_end(*sg);
*sg = sg_next(*sg);
}
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 4bd6c06..9b83784 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -172,6 +172,22 @@ static inline void sg_mark_end(struct scatterlist *sg)
}
/**
+ * sg_unmark_end - Undo setting the end of the scatterlist
+ * @sg: SG entryScatterlist
+ *
+ * Description:
+ * Removes the termination marker from the given entry of the scatterlist.
+ *
+ **/
+static inline void sg_unmark_end(struct scatterlist *sg)
+{
+#ifdef CONFIG_DEBUG_SG
+ BUG_ON(sg->sg_magic != SG_MAGIC);
+#endif
+ sg->page_link &= ~0x02;
+}
+
+/**
* sg_phys - Return physical address of an sg entry
* @sg: SG entry
*
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 02/16] virtio_ring: virtqueue_add_sgs, to add multiple sgs.
virtio_scsi can really use this, to avoid the current hack of copying
the whole sg array. Some other things get slightly neater, too.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 144 ++++++++++++++++++++++++++++++------------
include/linux/virtio.h | 7 ++
2 files changed, 109 insertions(+), 42 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 245177c..27e31d3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -100,14 +100,16 @@ struct vring_virtqueue
/* Set up an indirect table of descriptors and add it to the queue. */
static int vring_add_indirect(struct vring_virtqueue *vq,
- struct scatterlist sg[],
- unsigned int out,
- unsigned int in,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
gfp_t gfp)
{
struct vring_desc *desc;
unsigned head;
- int i;
+ struct scatterlist *sg;
+ int i, n;
/*
* We require lowmem mappings for the descriptors because
@@ -116,25 +118,31 @@ static int vring_add_indirect(struct vring_virtqueue *vq,
*/
gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH);
- desc = kmalloc((out + in) * sizeof(struct vring_desc), gfp);
+ desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp);
if (!desc)
return -ENOMEM;
- /* Transfer entries from the sg list into the indirect page */
- for (i = 0; i < out; i++) {
- desc[i].flags = VRING_DESC_F_NEXT;
- desc[i].addr = sg_phys(sg);
- desc[i].len = sg->length;
- desc[i].next = i+1;
- sg++;
+ /* Transfer entries from the sg lists into the indirect page */
+ i = 0;
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ desc[i].flags = VRING_DESC_F_NEXT;
+ desc[i].addr = sg_phys(sg);
+ desc[i].len = sg->length;
+ desc[i].next = i+1;
+ i++;
+ }
}
- for (; i < (out + in); i++) {
- desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
- desc[i].addr = sg_phys(sg);
- desc[i].len = sg->length;
- desc[i].next = i+1;
- sg++;
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
+ desc[i].addr = sg_phys(sg);
+ desc[i].len = sg->length;
+ desc[i].next = i+1;
+ i++;
+ }
}
+ BUG_ON(i != total_sg);
/* Last one doesn't continue. */
desc[i-1].flags &= ~VRING_DESC_F_NEXT;
@@ -176,8 +184,48 @@ int virtqueue_add_buf(struct virtqueue *_vq,
void *data,
gfp_t gfp)
{
+ struct scatterlist *sgs[2];
+ unsigned int i;
+
+ sgs[0] = sg;
+ sgs[1] = sg + out;
+
+ /* Workaround until callers pass well-formed sgs. */
+ for (i = 0; i < out + in; i++)
+ sg_unmark_end(sg + i);
+
+ sg_mark_end(sg + out + in - 1);
+ if (out && in)
+ sg_mark_end(sg + out - 1);
+
+ return virtqueue_add_sgs(_vq, sgs, out ? 1 : 0, in ? 1 : 0, data, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_buf);
+
+/**
+ * virtqueue_add_sgs - expose buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sgs: array of terminated scatterlists.
+ * @out_num: the number of scatterlists readable by other side
+ * @in_num: the number of scatterlists which are writable (after readable ones)
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ */
+int virtqueue_add_sgs(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ gfp_t gfp)
+{
struct vring_virtqueue *vq = to_vvq(_vq);
- unsigned int i, avail, uninitialized_var(prev);
+ struct scatterlist *sg;
+ unsigned int i, n, avail, uninitialized_var(prev), total_sg;
int head;
START_USE(vq);
@@ -197,46 +245,58 @@ int virtqueue_add_buf(struct virtqueue *_vq,
}
#endif
+ /* Count them first. */
+ for (i = total_sg = 0; i < out_sgs + in_sgs; i++) {
+ struct scatterlist *sg;
+ for (sg = sgs[i]; sg; sg = sg_next(sg))
+ total_sg++;
+ }
+
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
- if (vq->indirect && (out + in) > 1 &&
vq->vq.num_free) {
- head = vring_add_indirect(vq, sg, out, in, gfp);
+ if (vq->indirect && total_sg > 1 && vq->vq.num_free)
{
+ head = vring_add_indirect(vq, sgs, total_sg, out_sgs, in_sgs,
+ gfp);
if (likely(head >= 0))
goto add_head;
}
- BUG_ON(out + in > vq->vring.num);
- BUG_ON(out + in == 0);
+ BUG_ON(total_sg > vq->vring.num);
+ BUG_ON(total_sg == 0);
- if (vq->vq.num_free < out + in) {
+ if (vq->vq.num_free < total_sg) {
pr_debug("Can't add buf len %i - avail = %i\n",
- out + in, vq->vq.num_free);
+ total_sg, vq->vq.num_free);
/* FIXME: for historical reasons, we force a notify here if
* there are outgoing parts to the buffer. Presumably the
* host should service the ring ASAP. */
- if (out)
+ if (out_sgs)
vq->notify(&vq->vq);
END_USE(vq);
return -ENOSPC;
}
/* We're about to use some buffers from the free list. */
- vq->vq.num_free -= out + in;
-
- head = vq->free_head;
- for (i = vq->free_head; out; i = vq->vring.desc[i].next, out--) {
- vq->vring.desc[i].flags = VRING_DESC_F_NEXT;
- vq->vring.desc[i].addr = sg_phys(sg);
- vq->vring.desc[i].len = sg->length;
- prev = i;
- sg++;
+ vq->vq.num_free -= total_sg;
+
+ head = i = vq->free_head;
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ vq->vring.desc[i].flags = VRING_DESC_F_NEXT;
+ vq->vring.desc[i].addr = sg_phys(sg);
+ vq->vring.desc[i].len = sg->length;
+ prev = i;
+ i = vq->vring.desc[i].next;
+ }
}
- for (; in; i = vq->vring.desc[i].next, in--) {
- vq->vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
- vq->vring.desc[i].addr = sg_phys(sg);
- vq->vring.desc[i].len = sg->length;
- prev = i;
- sg++;
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ vq->vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
+ vq->vring.desc[i].addr = sg_phys(sg);
+ vq->vring.desc[i].len = sg->length;
+ prev = i;
+ i = vq->vring.desc[i].next;
+ }
}
/* Last one doesn't continue. */
vq->vring.desc[prev].flags &= ~VRING_DESC_F_NEXT;
@@ -269,7 +329,7 @@ add_head:
return 0;
}
-EXPORT_SYMBOL_GPL(virtqueue_add_buf);
+EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
/**
* virtqueue_kick_prepare - first half of split virtqueue_kick call.
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index ff6714e..6eff15b 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -40,6 +40,13 @@ int virtqueue_add_buf(struct virtqueue *vq,
void *data,
gfp_t gfp);
+int virtqueue_add_sgs(struct virtqueue *vq,
+ struct scatterlist *sgs[],
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ gfp_t gfp);
+
void virtqueue_kick(struct virtqueue *vq);
bool virtqueue_kick_prepare(struct virtqueue *vq);
--
1.7.10.4
From: Paolo Bonzini <pbonzini at redhat.com>
Right now, both virtblk_add_req and virtblk_add_req_wait call
virtqueue_add_buf. To prepare for the next patches, abstract the call
to virtqueue_add_buf into a new function __virtblk_add_req, and include
the waiting logic directly in virtblk_add_req.
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Reviewed-by: Asias He <asias at redhat.com>
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/block/virtio_blk.c | 55 ++++++++++++++++----------------------------
1 file changed, 20 insertions(+), 35 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 8ad21a2..fd8a689 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -100,50 +100,39 @@ static inline struct virtblk_req *virtblk_alloc_req(struct
virtio_blk *vblk,
return vbr;
}
-static void virtblk_add_buf_wait(struct virtio_blk *vblk,
- struct virtblk_req *vbr,
- unsigned long out,
- unsigned long in)
+static inline int __virtblk_add_req(struct virtqueue *vq,
+ struct virtblk_req *vbr,
+ unsigned long out,
+ unsigned long in)
{
+ return virtqueue_add_buf(vq, vbr->sg, out, in, vbr, GFP_ATOMIC);
+}
+
+static void virtblk_add_req(struct virtblk_req *vbr,
+ unsigned int out, unsigned int in)
+{
+ struct virtio_blk *vblk = vbr->vblk;
DEFINE_WAIT(wait);
+ int ret;
- for (;;) {
+ spin_lock_irq(vblk->disk->queue->queue_lock);
+ while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr,
+ out, in)) < 0)) {
prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
TASK_UNINTERRUPTIBLE);
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ io_schedule();
spin_lock_irq(vblk->disk->queue->queue_lock);
- if (virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
- GFP_ATOMIC) < 0) {
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- io_schedule();
- } else {
- virtqueue_kick(vblk->vq);
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- break;
- }
+ finish_wait(&vblk->queue_wait, &wait);
}
- finish_wait(&vblk->queue_wait, &wait);
-}
-
-static inline void virtblk_add_req(struct virtblk_req *vbr,
- unsigned int out, unsigned int in)
-{
- struct virtio_blk *vblk = vbr->vblk;
-
- spin_lock_irq(vblk->disk->queue->queue_lock);
- if (unlikely(virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
- GFP_ATOMIC) < 0)) {
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- virtblk_add_buf_wait(vblk, vbr, out, in);
- return;
- }
virtqueue_kick(vblk->vq);
spin_unlock_irq(vblk->disk->queue->queue_lock);
}
-static int virtblk_bio_send_flush(struct virtblk_req *vbr)
+static void virtblk_bio_send_flush(struct virtblk_req *vbr)
{
unsigned int out = 0, in = 0;
@@ -155,11 +144,9 @@ static int virtblk_bio_send_flush(struct virtblk_req *vbr)
sg_set_buf(&vbr->sg[out + in++], &vbr->status,
sizeof(vbr->status));
virtblk_add_req(vbr, out, in);
-
- return 0;
}
-static int virtblk_bio_send_data(struct virtblk_req *vbr)
+static void virtblk_bio_send_data(struct virtblk_req *vbr)
{
struct virtio_blk *vblk = vbr->vblk;
unsigned int num, out = 0, in = 0;
@@ -188,8 +175,6 @@ static int virtblk_bio_send_data(struct virtblk_req *vbr)
}
virtblk_add_req(vbr, out, in);
-
- return 0;
}
static void virtblk_bio_send_data_work(struct work_struct *work)
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 04/16] virtio-blk: use virtqueue_start_buf on bio path
(This is a respin of Paolo Bonzini's patch, but it calls
virtqueue_add_sgs() instead of his multi-part API).
Move the creation of the request header and response footer to
__virtblk_add_req. vbr->sg only contains the data scatterlist,
the header/footer are added separately using virtqueue_add_sgs().
With this change, virtio-blk (with use_bio) is not relying anymore on
the virtio functions ignoring the end markers in a scatterlist.
The next patch will do the same for the other path.
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/block/virtio_blk.c | 58 ++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index fd8a689..e27bc6c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -62,6 +62,7 @@ struct virtblk_req
struct virtio_blk *vblk;
int flags;
u8 status;
+ int nents;
struct scatterlist sg[];
};
@@ -100,24 +101,36 @@ static inline struct virtblk_req *virtblk_alloc_req(struct
virtio_blk *vblk,
return vbr;
}
-static inline int __virtblk_add_req(struct virtqueue *vq,
- struct virtblk_req *vbr,
- unsigned long out,
- unsigned long in)
+static int __virtblk_add_req(struct virtqueue *vq,
+ struct virtblk_req *vbr)
{
- return virtqueue_add_buf(vq, vbr->sg, out, in, vbr, GFP_ATOMIC);
+ struct scatterlist hdr, tailer, *sgs[3];
+ unsigned int num_out = 0, num_in = 0;
+
+ sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
+ sgs[num_out++] = &hdr;
+
+ if (vbr->nents) {
+ if (vbr->out_hdr.type & VIRTIO_BLK_T_OUT)
+ sgs[num_out++] = vbr->sg;
+ else
+ sgs[num_out + num_in++] = vbr->sg;
+ }
+
+ sg_init_one(&tailer, &vbr->status, sizeof(vbr->status));
+ sgs[num_out + num_in++] = &tailer;
+
+ return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
}
-static void virtblk_add_req(struct virtblk_req *vbr,
- unsigned int out, unsigned int in)
+static void virtblk_add_req(struct virtblk_req *vbr)
{
struct virtio_blk *vblk = vbr->vblk;
DEFINE_WAIT(wait);
int ret;
spin_lock_irq(vblk->disk->queue->queue_lock);
- while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr,
- out, in)) < 0)) {
+ while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr)) < 0)) {
prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
TASK_UNINTERRUPTIBLE);
@@ -134,22 +147,18 @@ static void virtblk_add_req(struct virtblk_req *vbr,
static void virtblk_bio_send_flush(struct virtblk_req *vbr)
{
- unsigned int out = 0, in = 0;
-
vbr->flags |= VBLK_IS_FLUSH;
vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
vbr->out_hdr.sector = 0;
vbr->out_hdr.ioprio = 0;
- sg_set_buf(&vbr->sg[out++], &vbr->out_hdr,
sizeof(vbr->out_hdr));
- sg_set_buf(&vbr->sg[out + in++], &vbr->status,
sizeof(vbr->status));
+ vbr->nents = 0;
- virtblk_add_req(vbr, out, in);
+ virtblk_add_req(vbr);
}
static void virtblk_bio_send_data(struct virtblk_req *vbr)
{
struct virtio_blk *vblk = vbr->vblk;
- unsigned int num, out = 0, in = 0;
struct bio *bio = vbr->bio;
vbr->flags &= ~VBLK_IS_FLUSH;
@@ -157,24 +166,15 @@ static void virtblk_bio_send_data(struct virtblk_req *vbr)
vbr->out_hdr.sector = bio->bi_sector;
vbr->out_hdr.ioprio = bio_prio(bio);
- sg_set_buf(&vbr->sg[out++], &vbr->out_hdr,
sizeof(vbr->out_hdr));
-
- num = blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg + out);
-
- sg_set_buf(&vbr->sg[num + out + in++], &vbr->status,
- sizeof(vbr->status));
-
- if (num) {
- if (bio->bi_rw & REQ_WRITE) {
+ vbr->nents = blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg);
+ if (vbr->nents) {
+ if (bio->bi_rw & REQ_WRITE)
vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
- out += num;
- } else {
+ else
vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
- in += num;
- }
}
- virtblk_add_req(vbr, out, in);
+ virtblk_add_req(vbr);
}
static void virtblk_bio_send_data_work(struct work_struct *work)
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 05/16] virtio-blk: use virtqueue_add_sgs on req path
From: Paolo Bonzini <pbonzini at redhat.com>
(This is a respin of Paolo Bonzini's patch, but it calls
virtqueue_add_sgs() instead of his multi-part API).
This is similar to the previous patch, but a bit more radical
because the bio and req paths now share the buffer construction
code. Because the req path doesn't use vbr->sg, however, we
need to add a couple of arguments to __virtblk_add_req.
We also need to teach __virtblk_add_req how to build SCSI command
requests.
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/block/virtio_blk.c | 69 +++++++++++++++++++++-----------------------
1 file changed, 33 insertions(+), 36 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index e27bc6c..523a70f 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -102,19 +102,40 @@ static inline struct virtblk_req *virtblk_alloc_req(struct
virtio_blk *vblk,
}
static int __virtblk_add_req(struct virtqueue *vq,
- struct virtblk_req *vbr)
+ struct virtblk_req *vbr,
+ struct scatterlist *data_sg,
+ unsigned data_nents)
{
- struct scatterlist hdr, tailer, *sgs[3];
+ struct scatterlist hdr, tailer, cmd, sense, inhdr, *sgs[6];
unsigned int num_out = 0, num_in = 0;
+ int type = vbr->out_hdr.type & ~VIRTIO_BLK_T_OUT;
sg_init_one(&hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
sgs[num_out++] = &hdr;
- if (vbr->nents) {
+ /*
+ * If this is a packet command we need a couple of additional headers.
+ * Behind the normal outhdr we put a segment with the scsi command
+ * block, and before the normal inhdr we put the sense data and the
+ * inhdr with additional status information.
+ */
+ if (type == VIRTIO_BLK_T_SCSI_CMD) {
+ sg_init_one(&cmd, vbr->req->cmd, vbr->req->cmd_len);
+ sgs[num_out++] = &cmd;
+ }
+
+ if (data_nents) {
if (vbr->out_hdr.type & VIRTIO_BLK_T_OUT)
- sgs[num_out++] = vbr->sg;
+ sgs[num_out++] = data_sg;
else
- sgs[num_out + num_in++] = vbr->sg;
+ sgs[num_out + num_in++] = data_sg;
+ }
+
+ if (type == VIRTIO_BLK_T_SCSI_CMD) {
+ sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+ sgs[num_out + num_in++] = &sense;
+ sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
+ sgs[num_out + num_in++] = &inhdr;
}
sg_init_one(&tailer, &vbr->status, sizeof(vbr->status));
@@ -130,7 +151,8 @@ static void virtblk_add_req(struct virtblk_req *vbr)
int ret;
spin_lock_irq(vblk->disk->queue->queue_lock);
- while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr)) < 0)) {
+ while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr, vbr->sg,
+ vbr->nents)) < 0)) {
prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
TASK_UNINTERRUPTIBLE);
@@ -283,7 +305,7 @@ static void virtblk_done(struct virtqueue *vq)
static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
struct request *req)
{
- unsigned long num, out = 0, in = 0;
+ unsigned int num;
struct virtblk_req *vbr;
vbr = virtblk_alloc_req(vblk, GFP_ATOMIC);
@@ -320,40 +342,15 @@ static bool do_req(struct request_queue *q, struct
virtio_blk *vblk,
}
}
- sg_set_buf(&vblk->sg[out++], &vbr->out_hdr,
sizeof(vbr->out_hdr));
-
- /*
- * If this is a packet command we need a couple of additional headers.
- * Behind the normal outhdr we put a segment with the scsi command
- * block, and before the normal inhdr we put the sense data and the
- * inhdr with additional status information before the normal inhdr.
- */
- if (vbr->req->cmd_type == REQ_TYPE_BLOCK_PC)
- sg_set_buf(&vblk->sg[out++], vbr->req->cmd,
vbr->req->cmd_len);
-
- num = blk_rq_map_sg(q, vbr->req, vblk->sg + out);
-
- if (vbr->req->cmd_type == REQ_TYPE_BLOCK_PC) {
- sg_set_buf(&vblk->sg[num + out + in++], vbr->req->sense,
SCSI_SENSE_BUFFERSIZE);
- sg_set_buf(&vblk->sg[num + out + in++], &vbr->in_hdr,
- sizeof(vbr->in_hdr));
- }
-
- sg_set_buf(&vblk->sg[num + out + in++], &vbr->status,
- sizeof(vbr->status));
-
+ num = blk_rq_map_sg(q, vbr->req, vblk->sg);
if (num) {
- if (rq_data_dir(vbr->req) == WRITE) {
+ if (rq_data_dir(vbr->req) == WRITE)
vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
- out += num;
- } else {
+ else
vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
- in += num;
- }
}
- if (virtqueue_add_buf(vblk->vq, vblk->sg, out, in, vbr,
- GFP_ATOMIC) < 0) {
+ if (__virtblk_add_req(vblk->vq, vbr, vblk->sg, num) < 0) {
mempool_free(vbr, vblk->pool);
return false;
}
--
1.7.10.4
It's simply a flag as to whether we have data now, so make it an
explicit function parameter rather than a member of struct
virtblk_req.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/block/virtio_blk.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 523a70f..6369cb9 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -62,7 +62,6 @@ struct virtblk_req
struct virtio_blk *vblk;
int flags;
u8 status;
- int nents;
struct scatterlist sg[];
};
@@ -104,7 +103,7 @@ static inline struct virtblk_req *virtblk_alloc_req(struct
virtio_blk *vblk,
static int __virtblk_add_req(struct virtqueue *vq,
struct virtblk_req *vbr,
struct scatterlist *data_sg,
- unsigned data_nents)
+ bool have_data)
{
struct scatterlist hdr, tailer, cmd, sense, inhdr, *sgs[6];
unsigned int num_out = 0, num_in = 0;
@@ -124,7 +123,7 @@ static int __virtblk_add_req(struct virtqueue *vq,
sgs[num_out++] = &cmd;
}
- if (data_nents) {
+ if (have_data) {
if (vbr->out_hdr.type & VIRTIO_BLK_T_OUT)
sgs[num_out++] = data_sg;
else
@@ -144,7 +143,7 @@ static int __virtblk_add_req(struct virtqueue *vq,
return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
}
-static void virtblk_add_req(struct virtblk_req *vbr)
+static void virtblk_add_req(struct virtblk_req *vbr, bool have_data)
{
struct virtio_blk *vblk = vbr->vblk;
DEFINE_WAIT(wait);
@@ -152,7 +151,7 @@ static void virtblk_add_req(struct virtblk_req *vbr)
spin_lock_irq(vblk->disk->queue->queue_lock);
while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr, vbr->sg,
- vbr->nents)) < 0)) {
+ have_data)) < 0)) {
prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
TASK_UNINTERRUPTIBLE);
@@ -173,30 +172,31 @@ static void virtblk_bio_send_flush(struct virtblk_req
*vbr)
vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
vbr->out_hdr.sector = 0;
vbr->out_hdr.ioprio = 0;
- vbr->nents = 0;
- virtblk_add_req(vbr);
+ virtblk_add_req(vbr, false);
}
static void virtblk_bio_send_data(struct virtblk_req *vbr)
{
struct virtio_blk *vblk = vbr->vblk;
struct bio *bio = vbr->bio;
+ bool have_data;
vbr->flags &= ~VBLK_IS_FLUSH;
vbr->out_hdr.type = 0;
vbr->out_hdr.sector = bio->bi_sector;
vbr->out_hdr.ioprio = bio_prio(bio);
- vbr->nents = blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg);
- if (vbr->nents) {
+ if (blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg)) {
+ have_data = true;
if (bio->bi_rw & REQ_WRITE)
vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
else
vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
- }
+ } else
+ have_data = false;
- virtblk_add_req(vbr);
+ virtblk_add_req(vbr, have_data);
}
static void virtblk_bio_send_data_work(struct work_struct *work)
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 07/16] virtio_ring: don't count elements twice for add_buf path.
Extract the post-counting code into virtqueue_add(), make both callers
use it.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 147 +++++++++++++++++++++++-------------------
1 file changed, 80 insertions(+), 67 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 27e31d3..c537385 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -163,69 +163,17 @@ static int vring_add_indirect(struct vring_virtqueue *vq,
return head;
}
-/**
- * virtqueue_add_buf - expose buffer to other end
- * @vq: the struct virtqueue we're talking about.
- * @sg: the description of the buffer(s).
- * @out_num: the number of sg readable by other side
- * @in_num: the number of sg which are writable (after readable ones)
- * @data: the token identifying the buffer.
- * @gfp: how to do memory allocations (if necessary).
- *
- * Caller must ensure we don't call this with other virtqueue operations
- * at the same time (except where noted).
- *
- * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
- */
-int virtqueue_add_buf(struct virtqueue *_vq,
- struct scatterlist sg[],
- unsigned int out,
- unsigned int in,
- void *data,
- gfp_t gfp)
-{
- struct scatterlist *sgs[2];
- unsigned int i;
-
- sgs[0] = sg;
- sgs[1] = sg + out;
-
- /* Workaround until callers pass well-formed sgs. */
- for (i = 0; i < out + in; i++)
- sg_unmark_end(sg + i);
-
- sg_mark_end(sg + out + in - 1);
- if (out && in)
- sg_mark_end(sg + out - 1);
-
- return virtqueue_add_sgs(_vq, sgs, out ? 1 : 0, in ? 1 : 0, data, gfp);
-}
-EXPORT_SYMBOL_GPL(virtqueue_add_buf);
-
-/**
- * virtqueue_add_sgs - expose buffers to other end
- * @vq: the struct virtqueue we're talking about.
- * @sgs: array of terminated scatterlists.
- * @out_num: the number of scatterlists readable by other side
- * @in_num: the number of scatterlists which are writable (after readable ones)
- * @data: the token identifying the buffer.
- * @gfp: how to do memory allocations (if necessary).
- *
- * Caller must ensure we don't call this with other virtqueue operations
- * at the same time (except where noted).
- *
- * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
- */
-int virtqueue_add_sgs(struct virtqueue *_vq,
- struct scatterlist *sgs[],
- unsigned int out_sgs,
- unsigned int in_sgs,
- void *data,
- gfp_t gfp)
+static int virtqueue_add(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ gfp_t gfp)
{
struct vring_virtqueue *vq = to_vvq(_vq);
struct scatterlist *sg;
- unsigned int i, n, avail, uninitialized_var(prev), total_sg;
+ unsigned int i, n, avail, uninitialized_var(prev);
int head;
START_USE(vq);
@@ -245,13 +193,6 @@ int virtqueue_add_sgs(struct virtqueue *_vq,
}
#endif
- /* Count them first. */
- for (i = total_sg = 0; i < out_sgs + in_sgs; i++) {
- struct scatterlist *sg;
- for (sg = sgs[i]; sg; sg = sg_next(sg))
- total_sg++;
- }
-
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
if (vq->indirect && total_sg > 1 && vq->vq.num_free)
{
@@ -329,6 +270,78 @@ add_head:
return 0;
}
+
+/**
+ * virtqueue_add_buf - expose buffer to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: the description of the buffer(s).
+ * @out_num: the number of sg readable by other side
+ * @in_num: the number of sg which are writable (after readable ones)
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ */
+int virtqueue_add_buf(struct virtqueue *_vq,
+ struct scatterlist sg[],
+ unsigned int out,
+ unsigned int in,
+ void *data,
+ gfp_t gfp)
+{
+ struct scatterlist *sgs[2];
+ unsigned int i;
+
+ sgs[0] = sg;
+ sgs[1] = sg + out;
+
+ /* Workaround until callers pass well-formed sgs. */
+ for (i = 0; i < out + in; i++)
+ sg_unmark_end(sg + i);
+
+ sg_mark_end(sg + out + in - 1);
+ if (out && in)
+ sg_mark_end(sg + out - 1);
+
+ return virtqueue_add(_vq, sgs, out+in, out ? 1 : 0, in ? 1 : 0,
+ data, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_buf);
+
+/**
+ * virtqueue_add_sgs - expose buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sgs: array of terminated scatterlists.
+ * @out_num: the number of scatterlists readable by other side
+ * @in_num: the number of scatterlists which are writable (after readable ones)
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ */
+int virtqueue_add_sgs(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ gfp_t gfp)
+{
+ unsigned int i, total_sg;
+
+ /* Count them first. */
+ for (i = total_sg = 0; i < out_sgs + in_sgs; i++) {
+ struct scatterlist *sg;
+ for (sg = sgs[i]; sg; sg = sg_next(sg))
+ total_sg++;
+ }
+ return virtqueue_add(_vq, sgs, total_sg, out_sgs, in_sgs, data, gfp);
+}
EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
/**
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 08/16] virtio_ring: virtqueue_add_outbuf / virtqueue_add_inbuf.
These are specialized versions of virtqueue_add_buf(), which cover
over 50% of cases and are far clearer.
In particular, the scatterlists passed to these functions don't have
to be clean (ie. we ignore end markers).
FIXME: I'm not sure about the unclean sglist bit. I had a more
ambitious one which conditionally ignored end markers in the iterator,
but it was ugly and I suspect this is just as fast. Maybe we should
just fix all the drivers?
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 58 ++++++++++++++++++++++++++++++++++++++++++
include/linux/virtio.h | 10 ++++++++
2 files changed, 68 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c537385..fb282b5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -345,6 +345,64 @@ int virtqueue_add_sgs(struct virtqueue *_vq,
EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
/**
+ * virtqueue_add_outbuf - expose output buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sgs: array of scatterlists (need not be terminated!)
+ * @num: the number of scatterlists readable by other side
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ */
+int virtqueue_add_outbuf(struct virtqueue *vq,
+ struct scatterlist sg[], unsigned int num,
+ void *data,
+ gfp_t gfp)
+{
+ unsigned int i;
+
+ /* We're about to iterate, so this is pretty cheap. */
+ for (i = 0; i < num - 1; i++)
+ sg_unmark_end(sg + i);
+ sg_mark_end(sg + i);
+
+ return virtqueue_add(vq, &sg, num, 1, 0, data, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
+
+/**
+ * virtqueue_add_inbuf - expose input buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sgs: array of scatterlists (need not be terminated!)
+ * @num: the number of scatterlists writable by other side
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ */
+int virtqueue_add_inbuf(struct virtqueue *vq,
+ struct scatterlist sg[], unsigned int num,
+ void *data,
+ gfp_t gfp)
+{
+ unsigned int i;
+
+ /* We're about to iterate, so this is pretty cheap. */
+ for (i = 0; i < num - 1; i++)
+ sg_unmark_end(sg + i);
+ sg_mark_end(sg + i);
+
+ return virtqueue_add(vq, &sg, num, 0, 1, data, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_inbuf);
+
+/**
* virtqueue_kick_prepare - first half of split virtqueue_kick call.
* @vq: the struct virtqueue
*
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 6eff15b..b442500 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -40,6 +40,16 @@ int virtqueue_add_buf(struct virtqueue *vq,
void *data,
gfp_t gfp);
+int virtqueue_add_outbuf(struct virtqueue *vq,
+ struct scatterlist sg[], unsigned int num,
+ void *data,
+ gfp_t gfp);
+
+int virtqueue_add_inbuf(struct virtqueue *vq,
+ struct scatterlist sg[], unsigned int num,
+ void *data,
+ gfp_t gfp);
+
int virtqueue_add_sgs(struct virtqueue *vq,
struct scatterlist *sgs[],
unsigned int out_sgs,
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 09/16] virtio_net: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index dbd5cd1..8ca69c0 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -443,7 +443,7 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp) skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); - err = virtqueue_add_buf(rq->vq, rq->sg, 0, 2, skb, gfp); + err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0) dev_kfree_skb(skb); @@ -488,8 +488,8 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp) /* chain first in list head */ first->private = (unsigned long)list; - err = virtqueue_add_buf(rq->vq, rq->sg, 0, MAX_SKB_FRAGS + 2, - first, gfp); + err = virtqueue_add_inbuf(rq->vq, rq->sg, MAX_SKB_FRAGS + 2, + first, gfp); if (err < 0) give_pages(rq, first); @@ -507,7 +507,7 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp) sg_init_one(rq->sg, page_address(page), PAGE_SIZE); - err = virtqueue_add_buf(rq->vq, rq->sg, 0, 1, page, gfp); + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, page, gfp); if (err < 0) give_pages(rq, page); @@ -710,8 +710,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) sg_set_buf(sq->sg, &hdr->hdr, sizeof hdr->hdr); num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1; - return virtqueue_add_buf(sq->vq, sq->sg, num_sg, - 0, skb, GFP_ATOMIC); + return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC); } static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev) -- 1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 10/16] virtio_net: use virtqueue_add_sgs[] for command buffers.
It's a bit cleaner to hand multiple sgs, rather than one big one.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/net/virtio_net.c | 50 ++++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8ca69c0..63ac5c1 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -39,7 +39,6 @@ module_param(gso, bool, 0444);
#define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
#define GOOD_COPY_LEN 128
-#define VIRTNET_SEND_COMMAND_SG_MAX 2
#define VIRTNET_DRIVER_VERSION "1.0.0"
struct virtnet_stats {
@@ -831,32 +830,32 @@ static void virtnet_netpoll(struct net_device *dev)
* never fail unless improperly formated.
*/
static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
- struct scatterlist *data, int out, int in)
+ struct scatterlist *out,
+ struct scatterlist *in)
{
- struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+ struct scatterlist *sgs[4], hdr, stat;
struct virtio_net_ctrl_hdr ctrl;
virtio_net_ctrl_ack status = ~0;
- unsigned int tmp;
- int i;
+ unsigned out_num = 0, in_num = 0, tmp;
/* Caller should know better */
- BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
- (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
- out++; /* Add header */
- in++; /* Add return status */
+ BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
- ctrl.class = class;
- ctrl.cmd = cmd;
+ /* Add header */
+ sg_init_one(&hdr, &ctrl, sizeof(ctrl));
+ sgs[out_num++] = &hdr;
- sg_init_table(sg, out + in);
+ if (out)
+ sgs[out_num++] = out;
+ if (in)
+ sgs[out_num + in_num++] = in;
- sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
- for_each_sg(data, s, out + in - 2, i)
- sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
- sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+ /* Add return status. */
+ sg_init_one(&stat, &status, sizeof(status));
+ sgs[out_num + in_num++] = &hdr;
- BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+ BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC)
+ < 0);
virtqueue_kick(vi->cvq);
@@ -874,8 +873,7 @@ static void virtnet_ack_link_announce(struct virtnet_info
*vi)
{
rtnl_lock();
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_ANNOUNCE,
- VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL,
- 0, 0))
+ VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, NULL))
dev_warn(&vi->dev->dev, "Failed to ack link
announce.\n");
rtnl_unlock();
}
@@ -893,7 +891,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16
queue_pairs)
sg_init_one(&sg, &s, sizeof(s));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
- VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg, 1, 0)){
+ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET, &sg, NULL)) {
dev_warn(&dev->dev, "Fail to set num of queue pairs to
%d\n",
queue_pairs);
return -EINVAL;
@@ -940,7 +938,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_PROMISC,
- sg, 1, 0))
+ sg, NULL))
dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
promisc ? "en" : "dis");
@@ -948,7 +946,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
VIRTIO_NET_CTRL_RX_ALLMULTI,
- sg, 1, 0))
+ sg, NULL))
dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
allmulti ? "en" : "dis");
@@ -987,7 +985,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
VIRTIO_NET_CTRL_MAC_TABLE_SET,
- sg, 2, 0))
+ sg, NULL))
dev_warn(&dev->dev, "Failed to set MAC fitler table.\n");
kfree(buf);
@@ -1001,7 +999,7 @@ static int virtnet_vlan_rx_add_vid(struct net_device *dev,
u16 vid)
sg_init_one(&sg, &vid, sizeof(vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
- VIRTIO_NET_CTRL_VLAN_ADD, &sg, 1, 0))
+ VIRTIO_NET_CTRL_VLAN_ADD, &sg, NULL))
dev_warn(&dev->dev, "Failed to add VLAN ID %d.\n", vid);
return 0;
}
@@ -1014,7 +1012,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device
*dev, u16 vid)
sg_init_one(&sg, &vid, sizeof(vid));
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_VLAN,
- VIRTIO_NET_CTRL_VLAN_DEL, &sg, 1, 0))
+ VIRTIO_NET_CTRL_VLAN_DEL, &sg, NULL))
dev_warn(&dev->dev, "Failed to kill VLAN ID %d.\n", vid);
return 0;
}
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 11/16] virtio_rng: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/char/hw_random/virtio-rng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c index 10fd71c..842e2d5 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -47,7 +47,7 @@ static void register_buffer(u8 *buf, size_t size) sg_init_one(&sg, buf, size); /* There should always be room for one buffer. */ - if (virtqueue_add_buf(vq, &sg, 0, 1, buf, GFP_KERNEL) < 0) + if (virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL) < 0) BUG(); virtqueue_kick(vq); -- 1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 12/16] virtio_console: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/char/virtio_console.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e905d5f..6d59f16 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -502,7 +502,7 @@ static int add_inbuf(struct virtqueue *vq, struct
port_buffer *buf)
sg_init_one(sg, buf->buf, buf->size);
- ret = virtqueue_add_buf(vq, sg, 0, 1, buf, GFP_ATOMIC);
+ ret = virtqueue_add_inbuf(vq, sg, 1, buf, GFP_ATOMIC);
virtqueue_kick(vq);
if (!ret)
ret = vq->num_free;
@@ -569,7 +569,7 @@ static ssize_t __send_control_msg(struct ports_device
*portdev, u32 port_id,
vq = portdev->c_ovq;
sg_init_one(sg, &cpkt, sizeof(cpkt));
- if (virtqueue_add_buf(vq, sg, 1, 0, &cpkt, GFP_ATOMIC) == 0) {
+ if (virtqueue_add_outbuf(vq, sg, 1, &cpkt, GFP_ATOMIC) == 0) {
virtqueue_kick(vq);
while (!virtqueue_get_buf(vq, &len))
cpu_relax();
@@ -618,7 +618,7 @@ static ssize_t __send_to_port(struct port *port, struct
scatterlist *sg,
reclaim_consumed_buffers(port);
- err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC);
+ err = virtqueue_add_outbuf(out_vq, sg, nents, data, GFP_ATOMIC);
/* Tell Host to go! */
virtqueue_kick(out_vq);
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 13/16] caif_virtio: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/net/caif/caif_virtio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index df832e7..9360055 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -364,8 +364,7 @@ static int cfv_netdev_tx(struct sk_buff *skb, struct
net_device *netdev)
/* Add buffer to avail ring. Flow control below should ensure
* that this always succeedes
*/
- ret = virtqueue_add_buf(cfv->vq_tx, &sg, 1, 0,
- buf_info, GFP_ATOMIC);
+ ret = virtqueue_add_outbuf(cfv->vq_tx, &sg, 1, buf_info, GFP_ATOMIC);
if (unlikely(WARN_ON(ret < 0))) {
kfree(buf_info);
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 14/16] virtio_rpmsg_bus: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index f1e3239..d9897d1 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -763,14 +763,14 @@ int rpmsg_send_offchannel_raw(struct rpmsg_channel *rpdev,
u32 src, u32 dst,
mutex_lock(&vrp->tx_lock);
/* add message to the remote processor's virtqueue */
- err = virtqueue_add_buf(vrp->svq, &sg, 1, 0, msg, GFP_KERNEL);
+ err = virtqueue_add_outbuf(vrp->svq, &sg, 1, msg, GFP_KERNEL);
if (err) {
/*
* need to reclaim the buffer here, otherwise it's lost
* (memory won't leak, but rpmsg won't use it again for TX).
* this will wait for a buffer management overhaul.
*/
- dev_err(dev, "virtqueue_add_buf failed: %d\n", err);
+ dev_err(dev, "virtqueue_add_outbuf failed: %d\n", err);
goto out;
}
@@ -845,7 +845,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
sg_init_one(&sg, msg, RPMSG_BUF_SIZE);
/* add the buffer back to the remote processor's virtqueue */
- err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, msg, GFP_KERNEL);
+ err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
if (err < 0) {
dev_err(dev, "failed to add a virtqueue buffer: %d\n", err);
return;
@@ -976,7 +976,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
sg_init_one(&sg, cpu_addr, RPMSG_BUF_SIZE);
- err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, cpu_addr,
+ err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
GFP_KERNEL);
WARN_ON(err); /* sanity check; this can't really happen */
}
--
1.7.10.4
Rusty Russell
2013-Feb-19 07:56 UTC
[PATCH 15/16] virtio_balloon: use simplified virtqueue accessors.
We never add buffers with input and output parts, so use the new accessors. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/virtio/virtio_balloon.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 8dab163..bd3ae32 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -108,7 +108,7 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns); /* We should always be able to add one buffer to an empty queue. */ - if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) + if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) BUG(); virtqueue_kick(vq); @@ -256,7 +256,7 @@ static void stats_handle_request(struct virtio_balloon *vb) if (!virtqueue_get_buf(vq, &len)) return; sg_init_one(&sg, vb->stats, sizeof(vb->stats)); - if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) + if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) BUG(); virtqueue_kick(vq); } @@ -341,7 +341,7 @@ static int init_vqs(struct virtio_balloon *vb) * use it to signal us later. */ sg_init_one(&sg, vb->stats, sizeof vb->stats); - if (virtqueue_add_buf(vb->stats_vq, &sg, 1, 0, vb, GFP_KERNEL) + if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL) < 0) BUG(); virtqueue_kick(vb->stats_vq); -- 1.7.10.4
virtio_add_buf() is going away, replaced with virtio_add_sgs() which
takes multiple terminated scatterlists.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
net/9p/trans_virtio.c | 48 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 39 insertions(+), 9 deletions(-)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index fd05c81..11d5f46 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -194,11 +194,14 @@ static int pack_sg_list(struct scatterlist *sg, int start,
if (s > count)
s = count;
BUG_ON(index > limit);
+ /* Make sure we don't terminate early. */
+ sg_unmark_end(&sg[index]);
sg_set_buf(&sg[index++], data, s);
count -= s;
data += s;
}
-
+ if (index-start)
+ sg_mark_end(&sg[index - 1]);
return index-start;
}
@@ -236,12 +239,17 @@ pack_sg_list_p(struct scatterlist *sg, int start, int
limit,
s = rest_of_page(data);
if (s > count)
s = count;
+ /* Make sure we don't terminate early. */
+ sg_unmark_end(&sg[index]);
sg_set_page(&sg[index++], pdata[i++], s, data_off);
data_off = 0;
data += s;
count -= s;
nr_pages--;
}
+
+ if (index-start)
+ sg_mark_end(&sg[index - 1]);
return index - start;
}
@@ -256,9 +264,10 @@ static int
p9_virtio_request(struct p9_client *client, struct p9_req_t *req)
{
int err;
- int in, out;
+ int in, out, out_sgs, in_sgs;
unsigned long flags;
struct virtio_chan *chan = client->trans;
+ struct scatterlist *sgs[2];
p9_debug(P9_DEBUG_TRANS, "9p debug: virtio request\n");
@@ -266,15 +275,21 @@ p9_virtio_request(struct p9_client *client, struct
p9_req_t *req)
req_retry:
spin_lock_irqsave(&chan->lock, flags);
+ out_sgs = in_sgs = 0;
/* Handle out VirtIO ring buffers */
out = pack_sg_list(chan->sg, 0,
VIRTQUEUE_NUM, req->tc->sdata, req->tc->size);
+ if (out)
+ sgs[out_sgs++] = chan->sg;
in = pack_sg_list(chan->sg, out,
VIRTQUEUE_NUM, req->rc->sdata, req->rc->capacity);
+ if (in)
+ sgs[out_sgs + in_sgs++] = chan->sg + out;
- err = virtqueue_add_buf(chan->vq, chan->sg, out, in, req->tc,
+ err = virtqueue_add_sgs(chan->vq, sgs, out_sgs, in_sgs, req->tc,
GFP_ATOMIC);
+
if (err < 0) {
if (err == -ENOSPC) {
chan->ring_bufs_avail = 0;
@@ -289,7 +304,7 @@ req_retry:
} else {
spin_unlock_irqrestore(&chan->lock, flags);
p9_debug(P9_DEBUG_TRANS,
- "virtio rpc add_buf returned failure\n");
+ "virtio rpc add_sgs returned failure\n");
return -EIO;
}
}
@@ -351,11 +366,12 @@ p9_virtio_zc_request(struct p9_client *client, struct
p9_req_t *req,
char *uidata, char *uodata, int inlen,
int outlen, int in_hdr_len, int kern_buf)
{
- int in, out, err;
+ int in, out, err, out_sgs, in_sgs;
unsigned long flags;
int in_nr_pages = 0, out_nr_pages = 0;
struct page **in_pages = NULL, **out_pages = NULL;
struct virtio_chan *chan = client->trans;
+ struct scatterlist *sgs[4];
p9_debug(P9_DEBUG_TRANS, "virtio request\n");
@@ -396,13 +412,22 @@ p9_virtio_zc_request(struct p9_client *client, struct
p9_req_t *req,
req->status = REQ_STATUS_SENT;
req_retry_pinned:
spin_lock_irqsave(&chan->lock, flags);
+
+ out_sgs = in_sgs = 0;
+
/* out data */
out = pack_sg_list(chan->sg, 0,
VIRTQUEUE_NUM, req->tc->sdata, req->tc->size);
- if (out_pages)
+ if (out)
+ sgs[out_sgs++] = chan->sg + out;
+
+ if (out_pages) {
out += pack_sg_list_p(chan->sg, out, VIRTQUEUE_NUM,
out_pages, out_nr_pages, uodata, outlen);
+ sgs[out_sgs++] = chan->sg + out;
+ }
+
/*
* Take care of in data
* For example TREAD have 11.
@@ -412,11 +437,16 @@ req_retry_pinned:
*/
in = pack_sg_list(chan->sg, out,
VIRTQUEUE_NUM, req->rc->sdata, in_hdr_len);
- if (in_pages)
+ if (in)
+ sgs[out_sgs + in_sgs++] = chan->sg + out + in;
+
+ if (in_pages) {
in += pack_sg_list_p(chan->sg, out + in, VIRTQUEUE_NUM,
in_pages, in_nr_pages, uidata, inlen);
+ sgs[out_sgs + in_sgs++] = chan->sg + out + in;
+ }
- err = virtqueue_add_buf(chan->vq, chan->sg, out, in, req->tc,
+ err = virtqueue_add_sgs(chan->vq, sgs, out_sgs, in_sgs, req->tc,
GFP_ATOMIC);
if (err < 0) {
if (err == -ENOSPC) {
@@ -432,7 +462,7 @@ req_retry_pinned:
} else {
spin_unlock_irqrestore(&chan->lock, flags);
p9_debug(P9_DEBUG_TRANS,
- "virtio rpc add_buf returned failure\n");
+ "virtio rpc add_sgs returned failure\n");
err = -EIO;
goto err_out;
}
--
1.7.10.4
Il 19/02/2013 08:56, Rusty Russell ha scritto:> OK, this is (ab)uses some of Paolo's patches. The first 7 are > candidates for this merge window (maybe), the rest I'm not so sure > about.Cool, thanks.> Thanks, > Rusty. > > Paolo Bonzini (3): > scatterlist: introduce sg_unmark_end > virtio-blk: reorganize virtblk_add_req > virtio-blk: use virtqueue_add_sgs on req path > > Rusty Russell (13): > virtio_ring: virtqueue_add_sgs, to add multiple sgs. > virtio-blk: use virtqueue_start_buf on bio pathSomething wrong with author and commit message in this patch. I'll send two follow-ups for virtio-scsi, one to be added before patch 7, one after. Paolo
Wanlong Gao
2013-Feb-20 08:37 UTC
[PATCH 17/16] virtio-scsi: use virtqueue_add_sgs for command buffers
Using the new virtqueue_add_sgs function lets us simplify the queueing
path. In particular, all data protected by the tgt_lock is just gone
(multiqueue will find a new use for the lock).
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
drivers/scsi/virtio_scsi.c | 93 ++++++++++++++++------------------------------
1 file changed, 33 insertions(+), 60 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 3449a1f..b002d7b 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -59,11 +59,8 @@ struct virtio_scsi_vq {
/* Per-target queue state */
struct virtio_scsi_target_state {
- /* Protects sg. Lock hierarchy is tgt_lock -> vq_lock. */
+ /* Never held at the same time as vq_lock. */
spinlock_t tgt_lock;
-
- /* For sglist construction when adding commands to the virtqueue. */
- struct scatterlist sg[];
};
/* Driver instance state */
@@ -351,75 +348,57 @@ static void virtscsi_event_done(struct virtqueue *vq)
spin_unlock_irqrestore(&vscsi->event_vq.vq_lock, flags);
};
-static void virtscsi_map_sgl(struct scatterlist *sg, unsigned int *p_idx,
- struct scsi_data_buffer *sdb)
-{
- struct sg_table *table = &sdb->table;
- struct scatterlist *sg_elem;
- unsigned int idx = *p_idx;
- int i;
-
- for_each_sg(table->sgl, sg_elem, table->nents, i)
- sg[idx++] = *sg_elem;
-
- *p_idx = idx;
-}
-
/**
- * virtscsi_map_cmd - map a scsi_cmd to a virtqueue scatterlist
- * @vscsi : virtio_scsi state
+ * virtscsi_add_cmd - add a virtio_scsi_cmd to a virtqueue
+ * @vq : the struct virtqueue we're talking about
* @cmd : command structure
- * @out_num : number of read-only elements
- * @in_num : number of write-only elements
* @req_size : size of the request buffer
* @resp_size : size of the response buffer
- *
- * Called with tgt_lock held.
+ * @gfp : flags to use for memory allocations
*/
-static void virtscsi_map_cmd(struct virtio_scsi_target_state *tgt,
- struct virtio_scsi_cmd *cmd,
- unsigned *out_num, unsigned *in_num,
- size_t req_size, size_t resp_size)
+static int virtscsi_add_cmd(struct virtqueue *vq,
+ struct virtio_scsi_cmd *cmd,
+ size_t req_size, size_t resp_size, gfp_t gfp)
{
struct scsi_cmnd *sc = cmd->sc;
- struct scatterlist *sg = tgt->sg;
- unsigned int idx = 0;
+ struct scatterlist *sgs[4], req, resp;
+ struct sg_table *out, *in;
+ unsigned out_num = 0, in_num = 0;
+
+ out = in = NULL;
- /* Request header. */
- sg_set_buf(&sg[idx++], &cmd->req, req_size);
+ if (sc && sc->sc_data_direction != DMA_NONE) {
+ if (sc->sc_data_direction != DMA_FROM_DEVICE)
+ out = &scsi_out(sc)->table;
+ if (sc->sc_data_direction != DMA_TO_DEVICE)
+ in = &scsi_in(sc)->table;
+ }
- /* Data-out buffer. */
- if (sc && sc->sc_data_direction != DMA_FROM_DEVICE)
- virtscsi_map_sgl(sg, &idx, scsi_out(sc));
+ sg_init_one(&req, &cmd->req, req_size);
+ sgs[out_num++] = &req;
- *out_num = idx;
+ if (out)
+ sgs[out_num++] = out->sgl;
- /* Response header. */
- sg_set_buf(&sg[idx++], &cmd->resp, resp_size);
+ sg_init_one(&resp, &cmd->resp, resp_size);
+ sgs[out_num + in_num++] = &resp;
- /* Data-in buffer */
- if (sc && sc->sc_data_direction != DMA_TO_DEVICE)
- virtscsi_map_sgl(sg, &idx, scsi_in(sc));
+ if (in)
+ sgs[out_num + in_num++] = in->sgl;
- *in_num = idx - *out_num;
+ return virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, gfp);
}
-static int virtscsi_kick_cmd(struct virtio_scsi_target_state *tgt,
- struct virtio_scsi_vq *vq,
+static int virtscsi_kick_cmd(struct virtio_scsi_vq *vq,
struct virtio_scsi_cmd *cmd,
size_t req_size, size_t resp_size, gfp_t gfp)
{
- unsigned int out_num, in_num;
unsigned long flags;
int err;
bool needs_kick = false;
- spin_lock_irqsave(&tgt->tgt_lock, flags);
- virtscsi_map_cmd(tgt, cmd, &out_num, &in_num, req_size, resp_size);
-
- spin_lock(&vq->vq_lock);
- err = virtqueue_add_buf(vq->vq, tgt->sg, out_num, in_num, cmd, gfp);
- spin_unlock(&tgt->tgt_lock);
+ spin_lock_irqsave(&vq->vq_lock, flags);
+ err = virtscsi_add_cmd(vq->vq, cmd, req_size, resp_size, gfp);
if (!err)
needs_kick = virtqueue_kick_prepare(vq->vq);
@@ -433,7 +412,6 @@ static int virtscsi_kick_cmd(struct virtio_scsi_target_state
*tgt,
static int virtscsi_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *sc)
{
struct virtio_scsi *vscsi = shost_priv(sh);
- struct virtio_scsi_target_state *tgt = vscsi->tgt[sc->device->id];
struct virtio_scsi_cmd *cmd;
int ret;
@@ -467,7 +445,7 @@ static int virtscsi_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *sc)
BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE);
memcpy(cmd->req.cmd.cdb, sc->cmnd, sc->cmd_len);
- if (virtscsi_kick_cmd(tgt, &vscsi->req_vq, cmd,
+ if (virtscsi_kick_cmd(&vscsi->req_vq, cmd,
sizeof cmd->req.cmd, sizeof cmd->resp.cmd,
GFP_ATOMIC) == 0)
ret = 0;
@@ -481,11 +459,10 @@ out:
static int virtscsi_tmf(struct virtio_scsi *vscsi, struct virtio_scsi_cmd *cmd)
{
DECLARE_COMPLETION_ONSTACK(comp);
- struct virtio_scsi_target_state *tgt =
vscsi->tgt[cmd->sc->device->id];
int ret = FAILED;
cmd->comp = ∁
- if (virtscsi_kick_cmd(tgt, &vscsi->ctrl_vq, cmd,
+ if (virtscsi_kick_cmd(&vscsi->ctrl_vq, cmd,
sizeof cmd->req.tmf, sizeof cmd->resp.tmf,
GFP_NOIO) < 0)
goto out;
@@ -591,15 +568,11 @@ static struct virtio_scsi_target_state
*virtscsi_alloc_tgt(
struct virtio_scsi_target_state *tgt;
gfp_t gfp_mask = GFP_KERNEL;
- /* We need extra sg elements at head and tail. */
- tgt = kmalloc(sizeof(*tgt) + sizeof(tgt->sg[0]) * (sg_elems + 2),
- gfp_mask);
-
+ tgt = kmalloc(sizeof(*tgt), gfp_mask);
if (!tgt)
return NULL;
spin_lock_init(&tgt->tgt_lock);
- sg_init_table(tgt->sg, sg_elems + 2);
return tgt;
}
--
1.8.1
Wanlong Gao
2013-Feb-20 09:47 UTC
[PATCH 17/16 V2] virtio-scsi: use virtqueue_add_sgs for command buffers
Using the new virtqueue_add_sgs function lets us simplify the queueing
path. In particular, all data protected by the tgt_lock is just gone
(multiqueue will find a new use for the lock).
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
V1->V2:
add the lost comments (Asias)
drivers/scsi/virtio_scsi.c | 91 +++++++++++++++++-----------------------------
1 file changed, 34 insertions(+), 57 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 3449a1f..bae0c95 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -59,11 +59,8 @@ struct virtio_scsi_vq {
/* Per-target queue state */
struct virtio_scsi_target_state {
- /* Protects sg. Lock hierarchy is tgt_lock -> vq_lock. */
+ /* Never held at the same time as vq_lock. */
spinlock_t tgt_lock;
-
- /* For sglist construction when adding commands to the virtqueue. */
- struct scatterlist sg[];
};
/* Driver instance state */
@@ -351,75 +348,61 @@ static void virtscsi_event_done(struct virtqueue *vq)
spin_unlock_irqrestore(&vscsi->event_vq.vq_lock, flags);
};
-static void virtscsi_map_sgl(struct scatterlist *sg, unsigned int *p_idx,
- struct scsi_data_buffer *sdb)
-{
- struct sg_table *table = &sdb->table;
- struct scatterlist *sg_elem;
- unsigned int idx = *p_idx;
- int i;
-
- for_each_sg(table->sgl, sg_elem, table->nents, i)
- sg[idx++] = *sg_elem;
-
- *p_idx = idx;
-}
-
/**
- * virtscsi_map_cmd - map a scsi_cmd to a virtqueue scatterlist
- * @vscsi : virtio_scsi state
+ * virtscsi_add_cmd - add a virtio_scsi_cmd to a virtqueue
+ * @vq : the struct virtqueue we're talking about
* @cmd : command structure
- * @out_num : number of read-only elements
- * @in_num : number of write-only elements
* @req_size : size of the request buffer
* @resp_size : size of the response buffer
- *
- * Called with tgt_lock held.
+ * @gfp : flags to use for memory allocations
*/
-static void virtscsi_map_cmd(struct virtio_scsi_target_state *tgt,
- struct virtio_scsi_cmd *cmd,
- unsigned *out_num, unsigned *in_num,
- size_t req_size, size_t resp_size)
+static int virtscsi_add_cmd(struct virtqueue *vq,
+ struct virtio_scsi_cmd *cmd,
+ size_t req_size, size_t resp_size, gfp_t gfp)
{
struct scsi_cmnd *sc = cmd->sc;
- struct scatterlist *sg = tgt->sg;
- unsigned int idx = 0;
+ struct scatterlist *sgs[4], req, resp;
+ struct sg_table *out, *in;
+ unsigned out_num = 0, in_num = 0;
+
+ out = in = NULL;
+
+ if (sc && sc->sc_data_direction != DMA_NONE) {
+ if (sc->sc_data_direction != DMA_FROM_DEVICE)
+ out = &scsi_out(sc)->table;
+ if (sc->sc_data_direction != DMA_TO_DEVICE)
+ in = &scsi_in(sc)->table;
+ }
/* Request header. */
- sg_set_buf(&sg[idx++], &cmd->req, req_size);
+ sg_init_one(&req, &cmd->req, req_size);
+ sgs[out_num++] = &req;
/* Data-out buffer. */
- if (sc && sc->sc_data_direction != DMA_FROM_DEVICE)
- virtscsi_map_sgl(sg, &idx, scsi_out(sc));
-
- *out_num = idx;
+ if (out)
+ sgs[out_num++] = out->sgl;
/* Response header. */
- sg_set_buf(&sg[idx++], &cmd->resp, resp_size);
+ sg_init_one(&resp, &cmd->resp, resp_size);
+ sgs[out_num + in_num++] = &resp;
/* Data-in buffer */
- if (sc && sc->sc_data_direction != DMA_TO_DEVICE)
- virtscsi_map_sgl(sg, &idx, scsi_in(sc));
+ if (in)
+ sgs[out_num + in_num++] = in->sgl;
- *in_num = idx - *out_num;
+ return virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, gfp);
}
-static int virtscsi_kick_cmd(struct virtio_scsi_target_state *tgt,
- struct virtio_scsi_vq *vq,
+static int virtscsi_kick_cmd(struct virtio_scsi_vq *vq,
struct virtio_scsi_cmd *cmd,
size_t req_size, size_t resp_size, gfp_t gfp)
{
- unsigned int out_num, in_num;
unsigned long flags;
int err;
bool needs_kick = false;
- spin_lock_irqsave(&tgt->tgt_lock, flags);
- virtscsi_map_cmd(tgt, cmd, &out_num, &in_num, req_size, resp_size);
-
- spin_lock(&vq->vq_lock);
- err = virtqueue_add_buf(vq->vq, tgt->sg, out_num, in_num, cmd, gfp);
- spin_unlock(&tgt->tgt_lock);
+ spin_lock_irqsave(&vq->vq_lock, flags);
+ err = virtscsi_add_cmd(vq->vq, cmd, req_size, resp_size, gfp);
if (!err)
needs_kick = virtqueue_kick_prepare(vq->vq);
@@ -433,7 +416,6 @@ static int virtscsi_kick_cmd(struct virtio_scsi_target_state
*tgt,
static int virtscsi_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *sc)
{
struct virtio_scsi *vscsi = shost_priv(sh);
- struct virtio_scsi_target_state *tgt = vscsi->tgt[sc->device->id];
struct virtio_scsi_cmd *cmd;
int ret;
@@ -467,7 +449,7 @@ static int virtscsi_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *sc)
BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE);
memcpy(cmd->req.cmd.cdb, sc->cmnd, sc->cmd_len);
- if (virtscsi_kick_cmd(tgt, &vscsi->req_vq, cmd,
+ if (virtscsi_kick_cmd(&vscsi->req_vq, cmd,
sizeof cmd->req.cmd, sizeof cmd->resp.cmd,
GFP_ATOMIC) == 0)
ret = 0;
@@ -481,11 +463,10 @@ out:
static int virtscsi_tmf(struct virtio_scsi *vscsi, struct virtio_scsi_cmd *cmd)
{
DECLARE_COMPLETION_ONSTACK(comp);
- struct virtio_scsi_target_state *tgt =
vscsi->tgt[cmd->sc->device->id];
int ret = FAILED;
cmd->comp = ∁
- if (virtscsi_kick_cmd(tgt, &vscsi->ctrl_vq, cmd,
+ if (virtscsi_kick_cmd(&vscsi->ctrl_vq, cmd,
sizeof cmd->req.tmf, sizeof cmd->resp.tmf,
GFP_NOIO) < 0)
goto out;
@@ -591,15 +572,11 @@ static struct virtio_scsi_target_state
*virtscsi_alloc_tgt(
struct virtio_scsi_target_state *tgt;
gfp_t gfp_mask = GFP_KERNEL;
- /* We need extra sg elements at head and tail. */
- tgt = kmalloc(sizeof(*tgt) + sizeof(tgt->sg[0]) * (sg_elems + 2),
- gfp_mask);
-
+ tgt = kmalloc(sizeof(*tgt), gfp_mask);
if (!tgt)
return NULL;
spin_lock_init(&tgt->tgt_lock);
- sg_init_table(tgt->sg, sg_elems + 2);
return tgt;
}
--
1.8.1
Wanlong Gao
2013-Feb-20 10:09 UTC
[PATCH 09/16] virtio_net: use simplified virtqueue accessors.
On 02/19/2013 03:56 PM, Rusty Russell wrote:> We never add buffers with input and output parts, so use the new accessors. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>Reviewed-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>> --- > drivers/net/virtio_net.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index dbd5cd1..8ca69c0 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -443,7 +443,7 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp) > > skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); > > - err = virtqueue_add_buf(rq->vq, rq->sg, 0, 2, skb, gfp); > + err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); > if (err < 0) > dev_kfree_skb(skb); > > @@ -488,8 +488,8 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp) > > /* chain first in list head */ > first->private = (unsigned long)list; > - err = virtqueue_add_buf(rq->vq, rq->sg, 0, MAX_SKB_FRAGS + 2, > - first, gfp); > + err = virtqueue_add_inbuf(rq->vq, rq->sg, MAX_SKB_FRAGS + 2, > + first, gfp); > if (err < 0) > give_pages(rq, first); > > @@ -507,7 +507,7 @@ static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp) > > sg_init_one(rq->sg, page_address(page), PAGE_SIZE); > > - err = virtqueue_add_buf(rq->vq, rq->sg, 0, 1, page, gfp); > + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, page, gfp); > if (err < 0) > give_pages(rq, page); > > @@ -710,8 +710,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) > sg_set_buf(sq->sg, &hdr->hdr, sizeof hdr->hdr); > > num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1; > - return virtqueue_add_buf(sq->vq, sq->sg, num_sg, > - 0, skb, GFP_ATOMIC); > + return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC); > } > > static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev) >
Wanlong Gao
2013-Feb-20 10:12 UTC
[PATCH 11/16] virtio_rng: use simplified virtqueue accessors.
On 02/19/2013 03:56 PM, Rusty Russell wrote:> We never add buffers with input and output parts, so use the new accessors. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>Reviewed-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>> --- > drivers/char/hw_random/virtio-rng.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c > index 10fd71c..842e2d5 100644 > --- a/drivers/char/hw_random/virtio-rng.c > +++ b/drivers/char/hw_random/virtio-rng.c > @@ -47,7 +47,7 @@ static void register_buffer(u8 *buf, size_t size) > sg_init_one(&sg, buf, size); > > /* There should always be room for one buffer. */ > - if (virtqueue_add_buf(vq, &sg, 0, 1, buf, GFP_KERNEL) < 0) > + if (virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL) < 0) > BUG(); > > virtqueue_kick(vq); >
Wanlong Gao
2013-Feb-20 10:12 UTC
[PATCH 12/16] virtio_console: use simplified virtqueue accessors.
On 02/19/2013 03:56 PM, Rusty Russell wrote:> We never add buffers with input and output parts, so use the new accessors. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>Reviewed-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>> --- > drivers/char/virtio_console.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index e905d5f..6d59f16 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -502,7 +502,7 @@ static int add_inbuf(struct virtqueue *vq, struct port_buffer *buf) > > sg_init_one(sg, buf->buf, buf->size); > > - ret = virtqueue_add_buf(vq, sg, 0, 1, buf, GFP_ATOMIC); > + ret = virtqueue_add_inbuf(vq, sg, 1, buf, GFP_ATOMIC); > virtqueue_kick(vq); > if (!ret) > ret = vq->num_free; > @@ -569,7 +569,7 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id, > vq = portdev->c_ovq; > > sg_init_one(sg, &cpkt, sizeof(cpkt)); > - if (virtqueue_add_buf(vq, sg, 1, 0, &cpkt, GFP_ATOMIC) == 0) { > + if (virtqueue_add_outbuf(vq, sg, 1, &cpkt, GFP_ATOMIC) == 0) { > virtqueue_kick(vq); > while (!virtqueue_get_buf(vq, &len)) > cpu_relax(); > @@ -618,7 +618,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, > > reclaim_consumed_buffers(port); > > - err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC); > + err = virtqueue_add_outbuf(out_vq, sg, nents, data, GFP_ATOMIC); > > /* Tell Host to go! */ > virtqueue_kick(out_vq); >
Wanlong Gao
2013-Feb-20 10:13 UTC
[PATCH 13/16] caif_virtio: use simplified virtqueue accessors.
On 02/19/2013 03:56 PM, Rusty Russell wrote:> We never add buffers with input and output parts, so use the new accessors. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>Reviewed-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>> --- > drivers/net/caif/caif_virtio.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c > index df832e7..9360055 100644 > --- a/drivers/net/caif/caif_virtio.c > +++ b/drivers/net/caif/caif_virtio.c > @@ -364,8 +364,7 @@ static int cfv_netdev_tx(struct sk_buff *skb, struct net_device *netdev) > /* Add buffer to avail ring. Flow control below should ensure > * that this always succeedes > */ > - ret = virtqueue_add_buf(cfv->vq_tx, &sg, 1, 0, > - buf_info, GFP_ATOMIC); > + ret = virtqueue_add_outbuf(cfv->vq_tx, &sg, 1, buf_info, GFP_ATOMIC); > > if (unlikely(WARN_ON(ret < 0))) { > kfree(buf_info); >
Wanlong Gao
2013-Feb-20 10:15 UTC
[PATCH 15/16] virtio_balloon: use simplified virtqueue accessors.
On 02/19/2013 03:56 PM, Rusty Russell wrote:> We never add buffers with input and output parts, so use the new accessors. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>Reviewed-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>> --- > drivers/virtio/virtio_balloon.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 8dab163..bd3ae32 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -108,7 +108,7 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq) > sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns); > > /* We should always be able to add one buffer to an empty queue. */ > - if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) > + if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) > BUG(); > virtqueue_kick(vq); > > @@ -256,7 +256,7 @@ static void stats_handle_request(struct virtio_balloon *vb) > if (!virtqueue_get_buf(vq, &len)) > return; > sg_init_one(&sg, vb->stats, sizeof(vb->stats)); > - if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0) > + if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0) > BUG(); > virtqueue_kick(vq); > } > @@ -341,7 +341,7 @@ static int init_vqs(struct virtio_balloon *vb) > * use it to signal us later. > */ > sg_init_one(&sg, vb->stats, sizeof vb->stats); > - if (virtqueue_add_buf(vb->stats_vq, &sg, 1, 0, vb, GFP_KERNEL) > + if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL) > < 0) > BUG(); > virtqueue_kick(vb->stats_vq); >
Maybe Matching Threads
- [PATCH 00/16] virtio ring rework.
- [PATCH 0/9] virtio: new API for addition of buffers, scatterlist changes
- [PATCH 0/9] virtio: new API for addition of buffers, scatterlist changes
- [PATCH 00/22] virtqueue_add_sgs, virtqueue_add_outbuf, virtqueue_add_inbuf
- [PATCH 00/22] virtqueue_add_sgs, virtqueue_add_outbuf, virtqueue_add_inbuf