All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
@ 2023-04-20  5:20 peili.dev
  2023-04-20  7:51 ` kernel test robot
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: peili.dev @ 2023-04-20  5:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: eperezma, Pei Li

From: Pei Li <peili@andrew.cmu.edu>

Signed-off-by: Pei Li <peili@andrew.cmu.edu>
---
 drivers/vhost/vdpa.c             | 77 +++++++++++++++++++++++++++++++-
 include/uapi/linux/vhost.h       |  7 +++
 include/uapi/linux/vhost_types.h |  1 +
 tools/include/uapi/linux/vhost.h |  6 +++
 4 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 7be9d9d8f01c..5419db1dfb7a 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -29,7 +29,8 @@ enum {
 	VHOST_VDPA_BACKEND_FEATURES =
 	(1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
 	(1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
-	(1ULL << VHOST_BACKEND_F_IOTLB_ASID),
+	(1ULL << VHOST_BACKEND_F_IOTLB_ASID) |
+	(1ULL << VHOST_BACKEND_F_IOCTL_BATCH),
 };
 
 #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
@@ -521,6 +522,68 @@ static long vhost_vdpa_resume(struct vhost_vdpa *v)
 	return ops->resume(vdpa);
 }
 
+static long vhost_vdpa_vring_ioctl_batch(struct vhost_vdpa *v, unsigned int cmd,
+				   void __user *argp)
+{
+	struct vdpa_device *vdpa = v->vdpa;
+	const struct vdpa_config_ops *ops = vdpa->config;
+	struct vhost_virtqueue *vq;
+	struct vhost_vring_state s;
+
+	u32 idx, num, i;
+	long r;
+
+	r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
+	if (r < 0) {
+		return r;
+	}
+
+	num = array_index_nospec(num, v->nvqs);
+
+	struct vhost_vring_state states[num + 1];
+
+	if (copy_from_user(&states, argp, sizeof(states)))
+		return -EFAULT;
+
+	switch (cmd) {
+	case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
+		for (i = 1; i <= num; i++) {
+			i = array_index_nospec(i, num + 1);
+			idx = states[i].index;
+			if (idx >= v->nvqs)
+				return -ENOBUFS;
+
+			idx = array_index_nospec(idx, v->nvqs);
+
+			ops->set_vq_ready(vdpa, idx, 1);
+		}
+		return 0;
+	case VHOST_VDPA_GET_VRING_GROUP_BATCH:
+		if (!ops->get_vq_group)
+			return -EOPNOTSUPP;
+		
+		for (i = 1; i <= num; i++) {
+			i = array_index_nospec(i, num + 1);
+			idx = states[i].index;
+			if (idx >= v->nvqs)
+				return -ENOBUFS;
+			idx = array_index_nospec(idx, v->nvqs);
+			states[i].num = ops->get_vq_group(vdpa, idx);
+			if (states[i].num >= vdpa->ngroups)
+				return -EIO;
+		}
+
+		if (copy_to_user(argp, &states, sizeof(states)))
+			return -EFAULT;
+
+		return 0;
+	default:
+		r = ENOIOCTLCMD;
+	}
+
+	return r;
+}
+
 static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 				   void __user *argp)
 {
@@ -533,6 +596,13 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 	u32 idx;
 	long r;
 
+	switch (cmd) {
+		case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
+		// fall through
+		case VHOST_VDPA_GET_VRING_GROUP_BATCH:
+			return vhost_vdpa_vring_ioctl_batch(v, cmd, argp);
+	}
+
 	r = get_user(idx, (u32 __user *)argp);
 	if (r < 0)
 		return r;
@@ -630,7 +700,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 			return -EFAULT;
 		if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
 				 BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
-				 BIT_ULL(VHOST_BACKEND_F_RESUME)))
+				 BIT_ULL(VHOST_BACKEND_F_RESUME) |
+				 BIT_ULL(VHOST_BACKEND_F_IOCTL_BATCH)))
 			return -EOPNOTSUPP;
 		if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
 		     !vhost_vdpa_can_suspend(v))
@@ -638,6 +709,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 		if ((features & BIT_ULL(VHOST_BACKEND_F_RESUME)) &&
 		     !vhost_vdpa_can_resume(v))
 			return -EOPNOTSUPP;
+
 		vhost_set_backend_features(&v->vdev, features);
 		return 0;
 	}
@@ -691,6 +763,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 			features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND);
 		if (vhost_vdpa_can_resume(v))
 			features |= BIT_ULL(VHOST_BACKEND_F_RESUME);
+		features |= BIT_ULL(VHOST_BACKEND_F_IOCTL_BATCH);
 		if (copy_to_user(featurep, &features, sizeof(features)))
 			r = -EFAULT;
 		break;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 92e1b700b51c..edb8cc1b22c9 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -188,4 +188,11 @@
  */
 #define VHOST_VDPA_RESUME		_IO(VHOST_VIRTIO, 0x7E)
 
+
+#define VHOST_VDPA_SET_VRING_ENABLE_BATCH	_IOW(VHOST_VIRTIO, 0x7F, \
+					     struct vhost_vring_state)
+
+#define VHOST_VDPA_GET_VRING_GROUP_BATCH	_IOWR(VHOST_VIRTIO, 0x82, \
+					      struct vhost_vring_state)
+
 #endif
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index c5690a8992d8..ea232fbd436a 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -166,4 +166,5 @@ struct vhost_vdpa_iova_range {
 /* Device can be resumed */
 #define VHOST_BACKEND_F_RESUME  0x5
 
+#define VHOST_BACKEND_F_IOCTL_BATCH 0x6
 #endif
diff --git a/tools/include/uapi/linux/vhost.h b/tools/include/uapi/linux/vhost.h
index 92e1b700b51c..d0ce141688ba 100644
--- a/tools/include/uapi/linux/vhost.h
+++ b/tools/include/uapi/linux/vhost.h
@@ -188,4 +188,10 @@
  */
 #define VHOST_VDPA_RESUME		_IO(VHOST_VIRTIO, 0x7E)
 
+#define VHOST_VDPA_SET_VRING_ENABLE_BATCH	_IOW(VHOST_VIRTIO, 0x7F, \
+					     struct vhost_vring_state)
+
+#define VHOST_VDPA_GET_VRING_GROUP_BATCH	_IOWR(VHOST_VIRTIO, 0x82, \
+					      struct vhost_vring_state)
+
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
  2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
@ 2023-04-20  7:51 ` kernel test robot
  2023-04-20  8:16 ` Jason Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-20  7:51 UTC (permalink / raw)
  To: peili.dev; +Cc: oe-kbuild-all

Hi,

[This is a private test report for your RFC patch.]
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.3-rc7 next-20230419]
[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/peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20230420052026.1883230-1-peili.dev%40gmail.com
patch subject: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230420/202304201529.bfoHvKF4-lkp@intel.com/config)
compiler: m68k-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/ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
        git checkout ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        # 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=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304201529.bfoHvKF4-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/rcuwait.h:6,
                    from include/linux/percpu-rwsem.h:7,
                    from include/linux/fs.h:33,
                    from include/linux/huge_mm.h:8,
                    from include/linux/mm.h:855,
                    from drivers/vhost/vdpa.c:18:
   drivers/vhost/vdpa.c: In function 'vhost_vdpa_vring_ioctl_batch':
>> arch/m68k/include/asm/uaccess.h:162:24: error: invalid type argument of unary '*' (have 'unsigned int')
     162 |         switch (sizeof(*(ptr))) {                                       \
         |                        ^~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:122:24: error: invalid type argument of unary '*' (have 'unsigned int')
     122 |                 : "m" (*(ptr)), "i" (err));                             \
         |                        ^~~~~~
   arch/m68k/include/asm/uaccess.h:164:17: note: in expansion of macro '__get_user_asm'
     164 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u8, b, d, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:123:31: error: invalid type argument of unary '*' (have 'unsigned int')
     123 |         (x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val;  \
         |                               ^~~~~~
   arch/m68k/include/asm/uaccess.h:164:17: note: in expansion of macro '__get_user_asm'
     164 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u8, b, d, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:122:24: error: invalid type argument of unary '*' (have 'unsigned int')
     122 |                 : "m" (*(ptr)), "i" (err));                             \
         |                        ^~~~~~
   arch/m68k/include/asm/uaccess.h:167:17: note: in expansion of macro '__get_user_asm'
     167 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u16, w, r, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:123:31: error: invalid type argument of unary '*' (have 'unsigned int')
     123 |         (x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val;  \
         |                               ^~~~~~
   arch/m68k/include/asm/uaccess.h:167:17: note: in expansion of macro '__get_user_asm'
     167 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u16, w, r, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:122:24: error: invalid type argument of unary '*' (have 'unsigned int')
     122 |                 : "m" (*(ptr)), "i" (err));                             \
         |                        ^~~~~~
   arch/m68k/include/asm/uaccess.h:170:17: note: in expansion of macro '__get_user_asm'
     170 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u32, l, r, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:123:31: error: invalid type argument of unary '*' (have 'unsigned int')
     123 |         (x) = (__force typeof(*(ptr)))(__force unsigned long)__gu_val;  \
         |                               ^~~~~~
   arch/m68k/include/asm/uaccess.h:170:17: note: in expansion of macro '__get_user_asm'
     170 |                 __get_user_asm(MOVES, __gu_err, x, ptr, u32, l, r, -EFAULT); \
         |                 ^~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/m68k/include/asm/uaccess.h:131:28: error: invalid type argument of unary '*' (have 'unsigned int')
     131 |                 __typeof__(*(ptr)) t;                                   \
         |                            ^~~~~~
   arch/m68k/include/asm/uaccess.h:173:17: note: in expansion of macro '__get_user_asm8'
     173 |                 __get_user_asm8(MOVES, __gu_err, x, ptr);               \
         |                 ^~~~~~~~~~~~~~~
   arch/m68k/include/asm/uaccess.h:180:26: note: in expansion of macro '__get_user'
     180 | #define get_user(x, ptr) __get_user(x, ptr)
         |                          ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> drivers/vhost/vdpa.c:565:16: warning: ISO C90 forbids variable length array 'states' [-Wvla]
     565 |         struct vhost_vring_state states[num + 1];
         |                ^~~~~~~~~~~~~~~~~
>> drivers/vhost/vdpa.c:565:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     565 |         struct vhost_vring_state states[num + 1];
         |         ^~~~~~
   drivers/vhost/vdpa.c:553:34: warning: unused variable 's' [-Wunused-variable]
     553 |         struct vhost_vring_state s;
         |                                  ^
   drivers/vhost/vdpa.c:552:33: warning: unused variable 'vq' [-Wunused-variable]
     552 |         struct vhost_virtqueue *vq;
         |                                 ^~


vim +/states +565 drivers/vhost/vdpa.c

   546	
   547	static long vhost_vdpa_vring_ioctl_batch(struct vhost_vdpa *v, unsigned int cmd,
   548					   void __user *argp)
   549	{
   550		struct vdpa_device *vdpa = v->vdpa;
   551		const struct vdpa_config_ops *ops = vdpa->config;
   552		struct vhost_virtqueue *vq;
   553		struct vhost_vring_state s;
   554	
   555		u32 idx, num, i;
   556		long r;
   557	
   558		r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
   559		if (r < 0) {
   560			return r;
   561		}
   562	
   563		num = array_index_nospec(num, v->nvqs);
   564	
 > 565		struct vhost_vring_state states[num + 1];
   566	
   567		if (copy_from_user(&states, argp, sizeof(states)))
   568			return -EFAULT;
   569	
   570		switch (cmd) {
   571		case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
   572			for (i = 1; i <= num; i++) {
   573				i = array_index_nospec(i, num + 1);
   574				idx = states[i].index;
   575				if (idx >= v->nvqs)
   576					return -ENOBUFS;
   577	
   578				idx = array_index_nospec(idx, v->nvqs);
   579	
   580				ops->set_vq_ready(vdpa, idx, 1);
   581			}
   582			return 0;
   583		case VHOST_VDPA_GET_VRING_GROUP_BATCH:
   584			if (!ops->get_vq_group)
   585				return -EOPNOTSUPP;
   586			
   587			for (i = 1; i <= num; i++) {
   588				i = array_index_nospec(i, num + 1);
   589				idx = states[i].index;
   590				if (idx >= v->nvqs)
   591					return -ENOBUFS;
   592				idx = array_index_nospec(idx, v->nvqs);
   593				states[i].num = ops->get_vq_group(vdpa, idx);
   594				if (states[i].num >= vdpa->ngroups)
   595					return -EIO;
   596			}
   597	
   598			if (copy_to_user(argp, &states, sizeof(states)))
   599				return -EFAULT;
   600	
   601			return 0;
   602		default:
   603			r = ENOIOCTLCMD;
   604		}
   605	
   606		return r;
   607	}
   608	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
  2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
  2023-04-20  7:51 ` kernel test robot
@ 2023-04-20  8:16 ` Jason Wang
  2023-04-20  8:33 ` kernel test robot
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2023-04-20  8:16 UTC (permalink / raw)
  To: peili.dev; +Cc: linux-kernel, eperezma, Pei Li

On Thu, Apr 20, 2023 at 1:23 PM <peili.dev@gmail.com> wrote:
>
> From: Pei Li <peili@andrew.cmu.edu>
>
> Signed-off-by: Pei Li <peili@andrew.cmu.edu>

Let's cc maintainers and have a change log for this patch.

More can be found at Documentation/process/submitting-patches.rst.

> ---
>  drivers/vhost/vdpa.c             | 77 +++++++++++++++++++++++++++++++-
>  include/uapi/linux/vhost.h       |  7 +++
>  include/uapi/linux/vhost_types.h |  1 +
>  tools/include/uapi/linux/vhost.h |  6 +++
>  4 files changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 7be9d9d8f01c..5419db1dfb7a 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -29,7 +29,8 @@ enum {
>         VHOST_VDPA_BACKEND_FEATURES =
>         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
>         (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> -       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
> +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID) |
> +       (1ULL << VHOST_BACKEND_F_IOCTL_BATCH),
>  };
>
>  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
> @@ -521,6 +522,68 @@ static long vhost_vdpa_resume(struct vhost_vdpa *v)
>         return ops->resume(vdpa);
>  }
>
> +static long vhost_vdpa_vring_ioctl_batch(struct vhost_vdpa *v, unsigned int cmd,
> +                                  void __user *argp)
> +{

While at it, I'd rather go with a general method to accept batching of
arbitrary combinations of multiple ioctls.

Thanks

> +       struct vdpa_device *vdpa = v->vdpa;
> +       const struct vdpa_config_ops *ops = vdpa->config;
> +       struct vhost_virtqueue *vq;
> +       struct vhost_vring_state s;
> +
> +       u32 idx, num, i;
> +       long r;
> +
> +       r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
> +       if (r < 0) {
> +               return r;
> +       }
> +
> +       num = array_index_nospec(num, v->nvqs);
> +
> +       struct vhost_vring_state states[num + 1];
> +
> +       if (copy_from_user(&states, argp, sizeof(states)))
> +               return -EFAULT;
> +
> +       switch (cmd) {
> +       case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
> +               for (i = 1; i <= num; i++) {
> +                       i = array_index_nospec(i, num + 1);
> +                       idx = states[i].index;
> +                       if (idx >= v->nvqs)
> +                               return -ENOBUFS;
> +
> +                       idx = array_index_nospec(idx, v->nvqs);
> +
> +                       ops->set_vq_ready(vdpa, idx, 1);
> +               }
> +               return 0;
> +       case VHOST_VDPA_GET_VRING_GROUP_BATCH:
> +               if (!ops->get_vq_group)
> +                       return -EOPNOTSUPP;
> +
> +               for (i = 1; i <= num; i++) {
> +                       i = array_index_nospec(i, num + 1);
> +                       idx = states[i].index;
> +                       if (idx >= v->nvqs)
> +                               return -ENOBUFS;
> +                       idx = array_index_nospec(idx, v->nvqs);
> +                       states[i].num = ops->get_vq_group(vdpa, idx);
> +                       if (states[i].num >= vdpa->ngroups)
> +                               return -EIO;
> +               }
> +
> +               if (copy_to_user(argp, &states, sizeof(states)))
> +                       return -EFAULT;
> +
> +               return 0;
> +       default:
> +               r = ENOIOCTLCMD;
> +       }
> +
> +       return r;
> +}
> +
>  static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>                                    void __user *argp)
>  {
> @@ -533,6 +596,13 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>         u32 idx;
>         long r;
>
> +       switch (cmd) {
> +               case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
> +               // fall through
> +               case VHOST_VDPA_GET_VRING_GROUP_BATCH:
> +                       return vhost_vdpa_vring_ioctl_batch(v, cmd, argp);
> +       }
> +
>         r = get_user(idx, (u32 __user *)argp);
>         if (r < 0)
>                 return r;
> @@ -630,7 +700,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>                         return -EFAULT;
>                 if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
>                                  BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> -                                BIT_ULL(VHOST_BACKEND_F_RESUME)))
> +                                BIT_ULL(VHOST_BACKEND_F_RESUME) |
> +                                BIT_ULL(VHOST_BACKEND_F_IOCTL_BATCH)))
>                         return -EOPNOTSUPP;
>                 if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
>                      !vhost_vdpa_can_suspend(v))
> @@ -638,6 +709,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>                 if ((features & BIT_ULL(VHOST_BACKEND_F_RESUME)) &&
>                      !vhost_vdpa_can_resume(v))
>                         return -EOPNOTSUPP;
> +
>                 vhost_set_backend_features(&v->vdev, features);
>                 return 0;
>         }
> @@ -691,6 +763,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>                         features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND);
>                 if (vhost_vdpa_can_resume(v))
>                         features |= BIT_ULL(VHOST_BACKEND_F_RESUME);
> +               features |= BIT_ULL(VHOST_BACKEND_F_IOCTL_BATCH);
>                 if (copy_to_user(featurep, &features, sizeof(features)))
>                         r = -EFAULT;
>                 break;
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 92e1b700b51c..edb8cc1b22c9 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -188,4 +188,11 @@
>   */
>  #define VHOST_VDPA_RESUME              _IO(VHOST_VIRTIO, 0x7E)
>
> +
> +#define VHOST_VDPA_SET_VRING_ENABLE_BATCH      _IOW(VHOST_VIRTIO, 0x7F, \
> +                                            struct vhost_vring_state)
> +
> +#define VHOST_VDPA_GET_VRING_GROUP_BATCH       _IOWR(VHOST_VIRTIO, 0x82, \
> +                                             struct vhost_vring_state)
> +
>  #endif
> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
> index c5690a8992d8..ea232fbd436a 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -166,4 +166,5 @@ struct vhost_vdpa_iova_range {
>  /* Device can be resumed */
>  #define VHOST_BACKEND_F_RESUME  0x5
>
> +#define VHOST_BACKEND_F_IOCTL_BATCH 0x6
>  #endif
> diff --git a/tools/include/uapi/linux/vhost.h b/tools/include/uapi/linux/vhost.h
> index 92e1b700b51c..d0ce141688ba 100644
> --- a/tools/include/uapi/linux/vhost.h
> +++ b/tools/include/uapi/linux/vhost.h
> @@ -188,4 +188,10 @@
>   */
>  #define VHOST_VDPA_RESUME              _IO(VHOST_VIRTIO, 0x7E)
>
> +#define VHOST_VDPA_SET_VRING_ENABLE_BATCH      _IOW(VHOST_VIRTIO, 0x7F, \
> +                                            struct vhost_vring_state)
> +
> +#define VHOST_VDPA_GET_VRING_GROUP_BATCH       _IOWR(VHOST_VIRTIO, 0x82, \
> +                                             struct vhost_vring_state)
> +
>  #endif
> --
> 2.25.1
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
  2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
  2023-04-20  7:51 ` kernel test robot
  2023-04-20  8:16 ` Jason Wang
@ 2023-04-20  8:33 ` kernel test robot
  2023-04-20  8:43 ` kernel test robot
  2023-04-20 11:28 ` kernel test robot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-20  8:33 UTC (permalink / raw)
  To: peili.dev; +Cc: oe-kbuild-all

Hi,

[This is a private test report for your RFC patch.]
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.3-rc7 next-20230419]
[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/peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20230420052026.1883230-1-peili.dev%40gmail.com
patch subject: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230420/202304201619.AlZrqoj4-lkp@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/ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
        git checkout ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        # 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 <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304201619.AlZrqoj4-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/rcuwait.h:6,
                    from include/linux/percpu-rwsem.h:7,
                    from include/linux/fs.h:33,
                    from arch/loongarch/include/asm/elf.h:9,
                    from include/linux/elf.h:6,
                    from include/linux/module.h:19,
                    from drivers/vhost/vdpa.c:15:
   drivers/vhost/vdpa.c: In function 'vhost_vdpa_vring_ioctl_batch':
>> arch/loongarch/include/asm/uaccess.h:47:26: error: invalid type argument of unary '*' (have 'unsigned int')
      47 |         const __typeof__(*(ptr)) __user *__p = (ptr);                   \
         |                          ^~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> arch/loongarch/include/asm/uaccess.h:47:48: warning: initialization of 'const int *' from 'unsigned int' makes pointer from integer without a cast [-Wint-conversion]
      47 |         const __typeof__(*(ptr)) __user *__p = (ptr);                   \
         |                                                ^
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   drivers/vhost/vdpa.c:565:16: warning: ISO C90 forbids variable length array 'states' [-Wvla]
     565 |         struct vhost_vring_state states[num + 1];
         |                ^~~~~~~~~~~~~~~~~
   drivers/vhost/vdpa.c:565:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     565 |         struct vhost_vring_state states[num + 1];
         |         ^~~~~~
   drivers/vhost/vdpa.c:553:34: warning: unused variable 's' [-Wunused-variable]
     553 |         struct vhost_vring_state s;
         |                                  ^
   drivers/vhost/vdpa.c:552:33: warning: unused variable 'vq' [-Wunused-variable]
     552 |         struct vhost_virtqueue *vq;
         |                                 ^~


vim +47 arch/loongarch/include/asm/uaccess.h

be769645a2aef5 Huacai Chen 2022-05-31  26  
be769645a2aef5 Huacai Chen 2022-05-31  27  /*
be769645a2aef5 Huacai Chen 2022-05-31  28   * get_user: - Get a simple variable from user space.
be769645a2aef5 Huacai Chen 2022-05-31  29   * @x:	 Variable to store result.
be769645a2aef5 Huacai Chen 2022-05-31  30   * @ptr: Source address, in user space.
be769645a2aef5 Huacai Chen 2022-05-31  31   *
be769645a2aef5 Huacai Chen 2022-05-31  32   * Context: User context only. This function may sleep if pagefaults are
be769645a2aef5 Huacai Chen 2022-05-31  33   *          enabled.
be769645a2aef5 Huacai Chen 2022-05-31  34   *
be769645a2aef5 Huacai Chen 2022-05-31  35   * This macro copies a single simple variable from user space to kernel
be769645a2aef5 Huacai Chen 2022-05-31  36   * space.  It supports simple types like char and int, but not larger
be769645a2aef5 Huacai Chen 2022-05-31  37   * data types like structures or arrays.
be769645a2aef5 Huacai Chen 2022-05-31  38   *
be769645a2aef5 Huacai Chen 2022-05-31  39   * @ptr must have pointer-to-simple-variable type, and the result of
be769645a2aef5 Huacai Chen 2022-05-31  40   * dereferencing @ptr must be assignable to @x without a cast.
be769645a2aef5 Huacai Chen 2022-05-31  41   *
be769645a2aef5 Huacai Chen 2022-05-31  42   * Returns zero on success, or -EFAULT on error.
be769645a2aef5 Huacai Chen 2022-05-31  43   * On error, the variable @x is set to zero.
be769645a2aef5 Huacai Chen 2022-05-31  44   */
be769645a2aef5 Huacai Chen 2022-05-31  45  #define get_user(x, ptr) \
be769645a2aef5 Huacai Chen 2022-05-31  46  ({									\
be769645a2aef5 Huacai Chen 2022-05-31 @47  	const __typeof__(*(ptr)) __user *__p = (ptr);			\
be769645a2aef5 Huacai Chen 2022-05-31  48  									\
be769645a2aef5 Huacai Chen 2022-05-31  49  	might_fault();							\
be769645a2aef5 Huacai Chen 2022-05-31  50  	access_ok(__p, sizeof(*__p)) ? __get_user((x), __p) :		\
be769645a2aef5 Huacai Chen 2022-05-31  51  				       ((x) = 0, -EFAULT);		\
be769645a2aef5 Huacai Chen 2022-05-31  52  })
be769645a2aef5 Huacai Chen 2022-05-31  53  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
  2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
                   ` (2 preceding siblings ...)
  2023-04-20  8:33 ` kernel test robot
@ 2023-04-20  8:43 ` kernel test robot
  2023-04-20 11:28 ` kernel test robot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-20  8:43 UTC (permalink / raw)
  To: peili.dev; +Cc: llvm, oe-kbuild-all

Hi,

[This is a private test report for your RFC patch.]
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.3-rc7 next-20230419]
[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/peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20230420052026.1883230-1-peili.dev%40gmail.com
patch subject: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
config: i386-randconfig-a012-20230417 (https://download.01.org/0day-ci/archive/20230420/202304201654.2ewx6SeJ-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
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/ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
        git checkout ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/vhost/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304201654.2ewx6SeJ-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/vhost/vdpa.c:558:6: error: indirection requires pointer operand ('unsigned int' invalid)
           r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:130:43: note: expanded from macro 'get_user'
   #define get_user(x,ptr) ({ might_fault(); do_get_user_call(get_user,x,ptr); })
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:101:21: note: expanded from macro 'do_get_user_call'
           register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
                    ~~~~~~~~~~^~~~~~~
   arch/x86/include/asm/uaccess.h:72:13: note: expanded from macro '__inttype'
           __typefits(x,char,                      \
           ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:78:31: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                        ^
>> drivers/vhost/vdpa.c:558:6: error: indirection requires pointer operand ('unsigned int' invalid)
           r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:130:43: note: expanded from macro 'get_user'
   #define get_user(x,ptr) ({ might_fault(); do_get_user_call(get_user,x,ptr); })
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:101:21: note: expanded from macro 'do_get_user_call'
           register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
                    ~~~~~~~~~~^~~~~~~
   arch/x86/include/asm/uaccess.h:73:15: note: expanded from macro '__inttype'
             __typefits(x,short,                   \
             ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:78:31: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                        ^
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                                                          ^~~
>> drivers/vhost/vdpa.c:558:6: error: indirection requires pointer operand ('unsigned int' invalid)
           r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:130:43: note: expanded from macro 'get_user'
   #define get_user(x,ptr) ({ might_fault(); do_get_user_call(get_user,x,ptr); })
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:101:21: note: expanded from macro 'do_get_user_call'
           register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
                    ~~~~~~~~~~^~~~~~~
   arch/x86/include/asm/uaccess.h:74:17: note: expanded from macro '__inttype'
               __typefits(x,int,                   \
               ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:78:31: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                        ^
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                                                          ^
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                                                          ^~~
>> drivers/vhost/vdpa.c:558:6: error: indirection requires pointer operand ('unsigned int' invalid)
           r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:130:43: note: expanded from macro 'get_user'
   #define get_user(x,ptr) ({ might_fault(); do_get_user_call(get_user,x,ptr); })
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:101:21: note: expanded from macro 'do_get_user_call'
           register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
                    ~~~~~~~~~~^~~~~~~
   arch/x86/include/asm/uaccess.h:75:19: note: expanded from macro '__inttype'
                 __typefits(x,long,0ULL)))))
                 ~~~~~~~~~~~^~~~~~~~~~~~~~~
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                                                          ^
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
   arch/x86/include/asm/uaccess.h:78:65: note: expanded from macro '__typefits'
           __builtin_choose_expr(sizeof(x)<=sizeof(type),(unsigned type)0,not)
                                                                          ^~~
>> drivers/vhost/vdpa.c:558:6: error: indirection requires pointer operand ('unsigned int' invalid)
           r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:130:43: note: expanded from macro 'get_user'
   #define get_user(x,ptr) ({ might_fault(); do_get_user_call(get_user,x,ptr); })
                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/uaccess.h:108:28: note: expanded from macro 'do_get_user_call'
           (x) = (__force __typeof__(*(ptr))) __val_gu;                    \
                                     ^~~~~~
>> drivers/vhost/vdpa.c:565:34: warning: variable length array used [-Wvla]
           struct vhost_vring_state states[num + 1];
                                           ^~~~~~~
>> drivers/vhost/vdpa.c:565:27: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           struct vhost_vring_state states[num + 1];
                                    ^
   2 warnings and 5 errors generated.


vim +558 drivers/vhost/vdpa.c

   546	
   547	static long vhost_vdpa_vring_ioctl_batch(struct vhost_vdpa *v, unsigned int cmd,
   548					   void __user *argp)
   549	{
   550		struct vdpa_device *vdpa = v->vdpa;
   551		const struct vdpa_config_ops *ops = vdpa->config;
   552		struct vhost_virtqueue *vq;
   553		struct vhost_vring_state s;
   554	
   555		u32 idx, num, i;
   556		long r;
   557	
 > 558		r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
   559		if (r < 0) {
   560			return r;
   561		}
   562	
   563		num = array_index_nospec(num, v->nvqs);
   564	
 > 565		struct vhost_vring_state states[num + 1];
   566	
   567		if (copy_from_user(&states, argp, sizeof(states)))
   568			return -EFAULT;
   569	
   570		switch (cmd) {
   571		case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
   572			for (i = 1; i <= num; i++) {
   573				i = array_index_nospec(i, num + 1);
   574				idx = states[i].index;
   575				if (idx >= v->nvqs)
   576					return -ENOBUFS;
   577	
   578				idx = array_index_nospec(idx, v->nvqs);
   579	
   580				ops->set_vq_ready(vdpa, idx, 1);
   581			}
   582			return 0;
   583		case VHOST_VDPA_GET_VRING_GROUP_BATCH:
   584			if (!ops->get_vq_group)
   585				return -EOPNOTSUPP;
   586			
   587			for (i = 1; i <= num; i++) {
   588				i = array_index_nospec(i, num + 1);
   589				idx = states[i].index;
   590				if (idx >= v->nvqs)
   591					return -ENOBUFS;
   592				idx = array_index_nospec(idx, v->nvqs);
   593				states[i].num = ops->get_vq_group(vdpa, idx);
   594				if (states[i].num >= vdpa->ngroups)
   595					return -EIO;
   596			}
   597	
   598			if (copy_to_user(argp, &states, sizeof(states)))
   599				return -EFAULT;
   600	
   601			return 0;
   602		default:
   603			r = ENOIOCTLCMD;
   604		}
   605	
   606		return r;
   607	}
   608	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
  2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
                   ` (3 preceding siblings ...)
  2023-04-20  8:43 ` kernel test robot
@ 2023-04-20 11:28 ` kernel test robot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-20 11:28 UTC (permalink / raw)
  To: peili.dev; +Cc: oe-kbuild-all

Hi,

[This is a private test report for your RFC patch.]
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.3-rc7 next-20230419]
[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/peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20230420052026.1883230-1-peili.dev%40gmail.com
patch subject: [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl()
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20230420/202304201916.BgWsotQz-lkp@intel.com/config)
compiler: s390-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/ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review peili-dev-gmail-com/Reduce-vdpa-initialization-startup-overhead-for-ioctl/20230420-132556
        git checkout ab8e2356cbe7e460fb8d5e16d57db96737f000b7
        # 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=s390 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304201916.BgWsotQz-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/rcuwait.h:6,
                    from include/linux/percpu-rwsem.h:7,
                    from include/linux/fs.h:33,
                    from include/linux/compat.h:17,
                    from arch/s390/include/asm/elf.h:160,
                    from include/linux/elf.h:6,
                    from include/linux/module.h:19,
                    from drivers/vhost/vdpa.c:15:
   drivers/vhost/vdpa.c: In function 'vhost_vdpa_vring_ioctl_batch':
>> arch/s390/include/asm/uaccess.h:228:24: error: invalid type argument of unary '*' (have 'unsigned int')
     228 |         switch (sizeof(*(ptr))) {                                       \
         |                        ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:232:60: error: invalid type argument of unary '*' (have 'unsigned int')
     232 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> drivers/vhost/vdpa.c:558:68: warning: passing argument 2 of '__get_user_fn' makes pointer from integer without a cast [-Wint-conversion]
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
         |                                                                    |
         |                                                                    unsigned int
   arch/s390/include/asm/uaccess.h:232:48: note: in definition of macro '__get_user'
     232 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                ^~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:161:70: note: expected 'const void *' but argument is of type 'unsigned int'
     161 | static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsigned long size)
         |                                                   ~~~~~~~~~~~~~~~~~~~^~~
   arch/s390/include/asm/uaccess.h:233:44: error: invalid type argument of unary '*' (have 'unsigned int')
     233 |                 (x) = *(__force __typeof__(*(ptr)) *)&__x;              \
         |                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:239:60: error: invalid type argument of unary '*' (have 'unsigned int')
     239 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> drivers/vhost/vdpa.c:558:68: warning: passing argument 2 of '__get_user_fn' makes pointer from integer without a cast [-Wint-conversion]
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
         |                                                                    |
         |                                                                    unsigned int
   arch/s390/include/asm/uaccess.h:239:48: note: in definition of macro '__get_user'
     239 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                ^~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:161:70: note: expected 'const void *' but argument is of type 'unsigned int'
     161 | static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsigned long size)
         |                                                   ~~~~~~~~~~~~~~~~~~~^~~
   arch/s390/include/asm/uaccess.h:240:44: error: invalid type argument of unary '*' (have 'unsigned int')
     240 |                 (x) = *(__force __typeof__(*(ptr)) *)&__x;              \
         |                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:246:60: error: invalid type argument of unary '*' (have 'unsigned int')
     246 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> drivers/vhost/vdpa.c:558:68: warning: passing argument 2 of '__get_user_fn' makes pointer from integer without a cast [-Wint-conversion]
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
         |                                                                    |
         |                                                                    unsigned int
   arch/s390/include/asm/uaccess.h:246:48: note: in definition of macro '__get_user'
     246 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                ^~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:161:70: note: expected 'const void *' but argument is of type 'unsigned int'
     161 | static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsigned long size)
         |                                                   ~~~~~~~~~~~~~~~~~~~^~~
   arch/s390/include/asm/uaccess.h:247:44: error: invalid type argument of unary '*' (have 'unsigned int')
     247 |                 (x) = *(__force __typeof__(*(ptr)) *)&__x;              \
         |                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:253:60: error: invalid type argument of unary '*' (have 'unsigned int')
     253 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
>> drivers/vhost/vdpa.c:558:68: warning: passing argument 2 of '__get_user_fn' makes pointer from integer without a cast [-Wint-conversion]
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
         |                                                                    |
         |                                                                    unsigned int
   arch/s390/include/asm/uaccess.h:253:48: note: in definition of macro '__get_user'
     253 |                 __gu_err = __get_user_fn(&__x, ptr, sizeof(*(ptr)));    \
         |                                                ^~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   arch/s390/include/asm/uaccess.h:161:70: note: expected 'const void *' but argument is of type 'unsigned int'
     161 | static __always_inline int __get_user_fn(void *x, const void __user *ptr, unsigned long size)
         |                                                   ~~~~~~~~~~~~~~~~~~~^~~
   arch/s390/include/asm/uaccess.h:254:44: error: invalid type argument of unary '*' (have 'unsigned int')
     254 |                 (x) = *(__force __typeof__(*(ptr)) *)&__x;              \
         |                                            ^~~~~~
   arch/s390/include/asm/uaccess.h:267:9: note: in expansion of macro '__get_user'
     267 |         __get_user(x, ptr);                                             \
         |         ^~~~~~~~~~
   drivers/vhost/vdpa.c:558:13: note: in expansion of macro 'get_user'
     558 |         r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
         |             ^~~~~~~~
   drivers/vhost/vdpa.c:565:16: warning: ISO C90 forbids variable length array 'states' [-Wvla]
     565 |         struct vhost_vring_state states[num + 1];
         |                ^~~~~~~~~~~~~~~~~
   drivers/vhost/vdpa.c:565:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     565 |         struct vhost_vring_state states[num + 1];
         |         ^~~~~~
   drivers/vhost/vdpa.c:553:34: warning: unused variable 's' [-Wunused-variable]
     553 |         struct vhost_vring_state s;
         |                                  ^
   drivers/vhost/vdpa.c:552:33: warning: unused variable 'vq' [-Wunused-variable]
     552 |         struct vhost_virtqueue *vq;
         |                                 ^~


vim +/__get_user_fn +558 drivers/vhost/vdpa.c

   546	
   547	static long vhost_vdpa_vring_ioctl_batch(struct vhost_vdpa *v, unsigned int cmd,
   548					   void __user *argp)
   549	{
   550		struct vdpa_device *vdpa = v->vdpa;
   551		const struct vdpa_config_ops *ops = vdpa->config;
   552		struct vhost_virtqueue *vq;
   553		struct vhost_vring_state s;
   554	
   555		u32 idx, num, i;
   556		long r;
   557	
 > 558		r = get_user(num, ((struct vhost_vring_state __user *)argp)->num);
   559		if (r < 0) {
   560			return r;
   561		}
   562	
   563		num = array_index_nospec(num, v->nvqs);
   564	
   565		struct vhost_vring_state states[num + 1];
   566	
   567		if (copy_from_user(&states, argp, sizeof(states)))
   568			return -EFAULT;
   569	
   570		switch (cmd) {
   571		case VHOST_VDPA_SET_VRING_ENABLE_BATCH:
   572			for (i = 1; i <= num; i++) {
   573				i = array_index_nospec(i, num + 1);
   574				idx = states[i].index;
   575				if (idx >= v->nvqs)
   576					return -ENOBUFS;
   577	
   578				idx = array_index_nospec(idx, v->nvqs);
   579	
   580				ops->set_vq_ready(vdpa, idx, 1);
   581			}
   582			return 0;
   583		case VHOST_VDPA_GET_VRING_GROUP_BATCH:
   584			if (!ops->get_vq_group)
   585				return -EOPNOTSUPP;
   586			
   587			for (i = 1; i <= num; i++) {
   588				i = array_index_nospec(i, num + 1);
   589				idx = states[i].index;
   590				if (idx >= v->nvqs)
   591					return -ENOBUFS;
   592				idx = array_index_nospec(idx, v->nvqs);
   593				states[i].num = ops->get_vq_group(vdpa, idx);
   594				if (states[i].num >= vdpa->ngroups)
   595					return -EIO;
   596			}
   597	
   598			if (copy_to_user(argp, &states, sizeof(states)))
   599				return -EFAULT;
   600	
   601			return 0;
   602		default:
   603			r = ENOIOCTLCMD;
   604		}
   605	
   606		return r;
   607	}
   608	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-20 11:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-20  5:20 [RFC PATCH] Reduce vdpa initialization / startup overhead for ioctl() peili.dev
2023-04-20  7:51 ` kernel test robot
2023-04-20  8:16 ` Jason Wang
2023-04-20  8:33 ` kernel test robot
2023-04-20  8:43 ` kernel test robot
2023-04-20 11:28 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.