On 5/12/23 19:35, Michael S. Tsirkin wrote:
On Fri, May 12, 2023 at 07:09:40PM +0800, zhenwei pi wrote:
On 5/12/23 18:46, Michael S. Tsirkin wrote:
On Fri, May 12, 2023 at 05:46:17PM +0800, zhenwei pi wrote:
There is already a virtqueue abstract structure in virtio subsystem
(see struct virtqueue in include/linux/virtio.h), however the vring
based virtqueue is used only in the past years, the virtqueue related
methods mix much with vring(just look like the codes, virtqueue_xxx
functions are implemented in virtio_ring.c).
Abstract virtqueue related methods(see struct virtqueue_ops), and
separate virtqueue_xxx symbols from vring. This allows a non-vring
based transport in the future. With this change, the following symbols
are exported from virtio.ko instead of virtio_ring.ko:
virtqueue_add_sgs
virtqueue_add_outbuf
virtqueue_add_inbuf
virtqueue_add_inbuf_ctx
virtqueue_kick_prepare
virtqueue_notify
virtqueue_kick
virtqueue_enable_cb_prepare
virtqueue_enable_cb
virtqueue_enable_cb_delayed
virtqueue_disable_cb
virtqueue_poll
virtqueue_get_buf_ctx
virtqueue_get_buf
virtqueue_detach_unused_buf
virtqueue_get_vring_size
virtqueue_resize
virtqueue_is_broken
virtio_break_device
__virtio_unbreak_device
Cc: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
Signed-off-by: zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
---
drivers/virtio/virtio.c | 362 +++++++++++++++++++++++++++++++++++
drivers/virtio/virtio_ring.c | 282 +++++----------------------
include/linux/virtio.h | 29 +++
3 files changed, 443 insertions(+), 230 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 3893dc29eb26..8d8715a10f26 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -553,6 +553,368 @@ int virtio_device_restore(struct virtio_device *dev)
EXPORT_SYMBOL_GPL(virtio_device_restore);
#endif
+/**
+ * virtqueue_add_sgs - expose buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sgs: array of terminated scatterlists.
+ * @out_sgs: the number of scatterlists readable by other side
+ * @in_sgs: 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, EIO).
+ */
+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 = 0;
+
+ /* Count them first. */
+ for (i = 0; i < out_sgs + in_sgs; i++) {
+ struct scatterlist *sg;
+
+ for (sg = sgs[i]; sg; sg = sg_next(sg))
+ total_sg++;
+ }
+ return vq->ops->add_sgs(vq, sgs, total_sg, out_sgs, in_sgs,
+ data, NULL, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
Hmm this kind of indirection on data path is going to be costly
performance-wise especially when retpolines are in place.
Any data on this?
Hi,
1, What about moving these functions into virtio.h and declare them as
static inline?
This will do nothing to remove indirection.
2, what about moving method fields into struct virtqueue?
This gets rid of one level of indirection but the big problem
is indirect function call due to retpolines, this remains.
Then we have struct like:
struct virtqueue {
struct list_head list;
...
void *priv;
/* virtqueue specific operations */
int (*add_sgs)(struct virtqueue *vq, struct scatterlist *sgs[],
unsigned int total_sg,
unsigned int out_sgs, unsigned int in_sgs,
void *data, void *ctx, gfp_t gfp);
...
}
and functions like:
static inline int virtqueue_add_sgs(...)
{
unsigned int i, total_sg = 0;
/* Count them first. */
for (i = 0; i < out_sgs + in_sgs; i++) {
struct scatterlist *sg;
for (sg = sgs[i]; sg; sg = sg_next(sg))
total_sg++;
}
return vq->add_sgs(vq, sgs, total_sg, out_sgs, in_sgs,
data, NULL, gfp);
}
Maybe a flag in vq:
bool abstract; /* use ops to add/get bufs and kick */
and then
if (unlikely(vq->abstract))
return vq->ops->add_sgs(vq, sgs, total_sg, out_sgs, in_sgs,
data, NULL, gfp);
transport then just sets the ops if it wants abstract vqs,
and core then skips the vring.
If [1] is acceptable, we can also reduce changes in patch 'tools/virtio:
implement virtqueue in test'.
Yea that one shouldn't be there.
--
zhenwei pi
OK, I'll try and send a next version a few days later. Thanks!
--
zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
