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); >
Reasonably Related 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