Re: [PATCH 1/2] virtio: abstract virtqueue related methods
Hi zhenwei, kernel test robot noticed the following build warnings: [auto build test WARNING on mst-vhost/linux-next] [also build test WARNING on linus/master v6.4-rc1 next-20230512] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928 base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next patch link: https://lore.kernel.org/r/20230512094618.433707-2-pizhenwei%40bytedance.com patch subject: [PATCH 1/2] virtio: abstract virtqueue related methods reproduce: # https://github.com/intel-lab-lkp/linux/commit/372bc1a0371968752fe0f5ec6e81edee6f9c44dd git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928 git checkout 372bc1a0371968752fe0f5ec6e81edee6f9c44dd make menuconfig # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS make htmldocs If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202305140142.c0qqq9wz-...@intel.com/ All warnings (new ones prefixed by >>): >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_inbuf' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_outbuf' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_add_sgs' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_get_buf_ctx' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_disable_cb' not found >> ./drivers/virtio/virtio_ring.c:1: warning: 'virtqueue_enable_cb' not found vim +/virtqueue_add_inbuf +1 ./drivers/virtio/virtio_ring.c fd534e9b5fdcf9 Thomas Gleixner 2019-05-23 @1 // SPDX-License-Identifier: GPL-2.0-or-later 0a8a69dd77ddbd Rusty Russell 2007-10-22 2 /* Virtio ring implementation. 0a8a69dd77ddbd Rusty Russell 2007-10-22 3 * 0a8a69dd77ddbd Rusty Russell 2007-10-22 4 * Copyright 2007 Rusty Russell IBM Corporation 0a8a69dd77ddbd Rusty Russell 2007-10-22 5 */ 0a8a69dd77ddbd Rusty Russell 2007-10-22 6 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 7 #include e34f87256794b8 Rusty Russell 2008-07-25 8 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 9 #include 5a0e3ad6af8660 Tejun Heo 2010-03-24 10 #include b5a2c4f1996d1d Paul Gortmaker 2011-07-03 11 #include e93300b1afc7cd Rusty Russell 2012-01-12 12 #include 780bc7903a32ed Andy Lutomirski 2016-02-02 13 #include 88938359e2dfe1 Alexander Potapenko 2022-09-15 14 #include f8ce72632fa7ed Michael S. Tsirkin 2021-08-10 15 #include 78fe39872378b0 Andy Lutomirski 2016-02-02 16 #include 0a8a69dd77ddbd Rusty Russell 2007-10-22 17 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/2] virtio: abstract virtqueue related methods
Hi zhenwei,
kernel test robot noticed the following build errors:
[auto build test ERROR on mst-vhost/linux-next]
[also build test ERROR on linus/master v6.4-rc1 next-20230512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230512094618.433707-2-pizhenwei%40bytedance.com
patch subject: [PATCH 1/2] virtio: abstract virtqueue related methods
config: loongarch-allyesconfig
(https://download.01.org/0day-ci/archive/20230513/202305130012.lq2kto5c-...@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/372bc1a0371968752fe0f5ec6e81edee6f9c44dd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
zhenwei-pi/virtio-abstract-virtqueue-related-methods/20230512-174928
git checkout 372bc1a0371968752fe0f5ec6e81edee6f9c44dd
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=loongarch olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Link:
https://lore.kernel.org/oe-kbuild-all/202305130012.lq2kto5c-...@intel.com/
All errors (new ones prefixed by >>):
drivers/virtio/virtio.c: In function 'virtio_break_device':
>> drivers/virtio/virtio.c:893:24: error: 'struct virtqueue_ops' has no member
>> named '__builtin_loongarch_break'
893 | vq->ops->__break(vq);
|^~
vim +893 drivers/virtio/virtio.c
882
883 /*
884 * This should prevent the device from being used, allowing drivers to
885 * recover. You may need to grab appropriate locks to flush.
886 */
887 void virtio_break_device(struct virtio_device *dev)
888 {
889 struct virtqueue *vq;
890
891 spin_lock(&dev->vqs_list_lock);
892 list_for_each_entry(vq, &dev->vqs, list) {
> 893 vq->ops->__break(vq);
894 }
895 spin_unlock(&dev->vqs_list_lock);
896 }
897 EXPORT_SYMBOL_GPL(virtio_break_device);
898
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: Re: [PATCH 1/2] virtio: abstract virtqueue related methods
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
Signed-off-by: zhenwei pi
---
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!
--
zhen
Re: Re: [PATCH 1/2] virtio: abstract virtqueue related methods
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
> > > Signed-off-by: zhenwei pi
> > > ---
> > > 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_
Re: Re: [PATCH 1/2] virtio: abstract virtqueue related methods
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
Signed-off-by: zhenwei pi
---
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?
2, what about moving method fields into struct virtqueue?
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);
}
If [1] is acceptable, we can also reduce changes in patch 'tools/virtio:
implement virtqueue in test'.
--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/2] virtio: abstract virtqueue related methods
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
> Signed-off-by: zhenwei pi
> ---
> 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?
> +
> +/**
> + * virtqueue_add_outbuf - expose output buffers to other end
> + * @vq: the struct virtqueue we're talking about.
> + * @sg: scatterlist (must be well-formed and terminated!)
> + * @num: the number of entries in @sg 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, EIO).
> + */
> +int virtqueue_add_outbuf(struct virtqueue *vq, struct scatterlist *sg,
> + unsigned int num, void *data, gfp_t gfp)
> +{
> + return vq->ops->add_sgs(vq, &sg, num, 1, 0, data, NULL, gfp);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
> +
> +/**
> + * virtqueue_add_inbuf - expose input buffers to other end
> + * @vq: the struct virtqueue we're talking about.
> + * @sg: scatterlist (must be well-formed and terminated!)
> + * @num: the number of entries in @sg 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, EIO).
> + */
> +int virtqueue_add_inbuf(struct virtqueue *vq, struct scatterlist *sg,
> + unsigned int num, void *data, gfp_t gfp)
> +{
> + return vq->ops->add_sgs(vq, &sg, num, 0, 1, data, NULL, gfp);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_add_inbuf);
> +
> +/**
> + * virtqueue_add_inbuf_ctx - expose input buffers to other end
> + * @vq: the struct virtqueue we're talking about.
> + * @sg: scatterlist (must be well-formed and terminated!)
[PATCH 1/2] virtio: abstract virtqueue related methods
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
Signed-off-by: zhenwei pi
---
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);
+
+/**
+ * virtqueue_add_outbuf - expose output buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: scatterlist (must be well-formed and terminated!)
+ * @num: the number of entries in @sg 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, EIO).
+ */
+int virtqueue_add_outbuf(struct virtqueue *vq, struct scatterlist *sg,
+unsigned int num, void *data, gfp_t gfp)
+{
+ return vq->ops->add_sgs(vq, &sg, num, 1, 0, data, NULL, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
+
+/**
+ * virtqueue_add_inbuf - expose input buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: scatterlist (must be well-formed and terminated!)
+ * @num: the number of entries in @sg 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, EIO).
+ */
+int virtqueue_add_inbuf(struct virtqueue *vq, struct scatterlist *sg,
+ unsigned int num, void *data, gfp_t gfp)
+{
+ return vq->ops->add_sgs(vq, &sg, num, 0, 1, data, NULL, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_inbuf);
+
+/**
+ * virtqueue_add_inbuf_ctx - expose input buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: scatterlist (must be well-formed and terminated!)
+ * @num: the number of entries in @sg writable by other side
+ * @data: the token identifying the buffer.
+ * @ctx: extra context for the token
+ * @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_inbuf_ctx
