Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
On 2020/6/2 下午6:16, Jason Wang wrote:
On 2020/6/2 下午5:42, Dan Carpenter wrote:
Hi Zhu,
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vdpa-bypass-waking-up-vhost_woker-for-vdpa-vq-kick/20200526-133819
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
linux-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vdpa.c:348 vhost_vdpa_set_vring_kick() error:
uninitialized symbol 'r'.
#
https://github.com/0day-ci/linux/commit/a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
vim +/r +348 drivers/vhost/vdpa.c
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 316 static long
vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq,
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 317
void __user *argp)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 318 {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 319 bool pollstart =
false, pollstop = false;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 320 struct file
*eventfp, *filep = NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 321 struct
vhost_vring_file f;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 322 long r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 323
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 324 if
(copy_from_user(, argp, sizeof(f)))
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 325 return -EFAULT;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 326
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 327 eventfp = f.fd == -1
? NULL : eventfd_fget(f.fd);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 328 if (IS_ERR(eventfp)) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 329 r =
PTR_ERR(eventfp);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 330 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 331 }
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 332
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 333 if (eventfp !=
vq->kick) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 334 pollstop =
(filep = vq->kick) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 335 pollstart =
(vq->kick = eventfp) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 336 } else
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 337 filep = eventfp;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 338
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 339 if (pollstop &&
vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 340 vhost_vdpa_poll_stop(vq);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 341
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 342 if (filep)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 343 fput(filep);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 344
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 345 if (pollstart &&
vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 346 r =
vhost_vdpa_poll_start(vq);
"r" not initialized on else path.
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 347
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 @348 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 349 }
Will fix.
Thanks
Lingshan reminds me that we've posted V2 which reuses the vhost.c
implementation for polling.
So there's no need for the fix.
Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
On 2020/6/2 下午5:42, Dan Carpenter wrote:
Hi Zhu,
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vdpa-bypass-waking-up-vhost_woker-for-vdpa-vq-kick/20200526-133819
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vdpa.c:348 vhost_vdpa_set_vring_kick() error: uninitialized
symbol 'r'.
#
https://github.com/0day-ci/linux/commit/a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
vim +/r +348 drivers/vhost/vdpa.c
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 316 static long
vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq,
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 317
void __user *argp)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 318 {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 319 bool pollstart = false,
pollstop = false;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 320 struct file *eventfp, *filep =
NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 321 struct vhost_vring_file f;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 322 long r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 323
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 324 if (copy_from_user(, argp,
sizeof(f)))
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 325 return -EFAULT;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 326
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 327 eventfp = f.fd == -1 ? NULL :
eventfd_fget(f.fd);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 328 if (IS_ERR(eventfp)) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 329 r = PTR_ERR(eventfp);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 330 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 331 }
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 332
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 333 if (eventfp != vq->kick) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 334 pollstop = (filep =
vq->kick) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 335 pollstart = (vq->kick =
eventfp) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 336 } else
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 337 filep = eventfp;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 338
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 339 if (pollstop && vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 340
vhost_vdpa_poll_stop(vq);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 341
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 342 if (filep)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 343 fput(filep);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 344
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 345 if (pollstart &&
vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 346 r =
vhost_vdpa_poll_start(vq);
"r" not initialized on else path.
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 347
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 @348 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 349 }
Will fix.
Thanks
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
Hi Zhu,
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vdpa-bypass-waking-up-vhost_woker-for-vdpa-vq-kick/20200526-133819
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vdpa.c:348 vhost_vdpa_set_vring_kick() error: uninitialized
symbol 'r'.
#
https://github.com/0day-ci/linux/commit/a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
vim +/r +348 drivers/vhost/vdpa.c
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 316 static long
vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq,
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 317
void __user *argp)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 318 {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 319 bool pollstart = false,
pollstop = false;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 320 struct file *eventfp, *filep =
NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 321 struct vhost_vring_file f;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 322 long r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 323
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 324 if (copy_from_user(, argp,
sizeof(f)))
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 325 return -EFAULT;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 326
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 327 eventfp = f.fd == -1 ? NULL :
eventfd_fget(f.fd);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 328 if (IS_ERR(eventfp)) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 329 r = PTR_ERR(eventfp);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 330 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 331 }
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 332
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 333 if (eventfp != vq->kick) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 334 pollstop = (filep =
vq->kick) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 335 pollstart = (vq->kick =
eventfp) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 336 } else
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 337 filep = eventfp;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 338
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 339 if (pollstop && vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 340
vhost_vdpa_poll_stop(vq);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 341
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 342 if (filep)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 343 fput(filep);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 344
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 345 if (pollstart &&
vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 346 r =
vhost_vdpa_poll_start(vq);
"r" not initialized on else path.
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 347
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 @348 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 349 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
On 2020/5/26 下午1:32, Zhu Lingshan wrote:
Standard vhost devices rely on waking up a vhost_worker to kick
a virtquque. However vdpa devices have hardware backends, so it
does not need this waking up routin. In this commit, vdpa device
will kick a virtqueue directly, reduce the performance overhead
caused by waking up a vhost_woker.
Thanks for the patch. It would be helpful if you can share some
performance numbers.
And the title should be "vhost-vdpa:" instead of "vdpa:"
This patch is important since we want to get rid of ktrhead and
use_mm()/unuse_mm() stuffs which allows us to implement doorbell mapping.
Signed-off-by: Zhu Lingshan
Suggested-by: Jason Wang
---
drivers/vhost/vdpa.c | 100 +++
1 file changed, 100 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 0968361..d3a2aca 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -287,6 +287,66 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v,
u16 __user *argp)
return 0;
}
+void vhost_vdpa_poll_stop(struct vhost_virtqueue *vq)
+{
+ vhost_poll_stop(>poll);
+}
+
+int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
+{
+ struct vhost_poll *poll = >poll;
+ struct file *file = vq->kick;
+ __poll_t mask;
+
+
+ if (poll->wqh)
+ return 0;
+
+ mask = vfs_poll(file, >table);
+ if (mask)
+ vq->handle_kick(>poll.work);
+ if (mask & EPOLLERR) {
+ vhost_poll_stop(poll);
+ return -EINVAL;
+ }
+
+ return 0;
+}
So this basically a duplication of vhost_poll_start()?
+
+static long vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq,
+ void __user *argp)
+{
+ bool pollstart = false, pollstop = false;
+ struct file *eventfp, *filep = NULL;
+ struct vhost_vring_file f;
+ long r;
+
+ if (copy_from_user(, argp, sizeof(f)))
+ return -EFAULT;
+
+ eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
+ if (IS_ERR(eventfp)) {
+ r = PTR_ERR(eventfp);
+ return r;
+ }
+
+ if (eventfp != vq->kick) {
+ pollstop = (filep = vq->kick) != NULL;
+ pollstart = (vq->kick = eventfp) != NULL;
+ } else
+ filep = eventfp;
+
+ if (pollstop && vq->handle_kick)
+ vhost_vdpa_poll_stop(vq);
+
+ if (filep)
+ fput(filep);
+
+ if (pollstart && vq->handle_kick)
+ r = vhost_vdpa_poll_start(vq);
+
+ return r;
+}
static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
void __user *argp)
@@ -316,6 +376,11 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
return 0;
}
+ if (cmd == VHOST_SET_VRING_KICK) {
+ r = vhost_vdpa_set_vring_kick(vq, argp);
+ return r;
+ }
+
if (cmd == VHOST_GET_VRING_BASE)
vq->last_avail_idx = ops->get_vq_state(v->vdpa, idx);
@@ -667,6 +732,39 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v)
v->domain = NULL;
}
+static int vhost_vdpa_poll_worker(wait_queue_entry_t *wait, unsigned int mode,
+ int sync, void *key)
+{
+ struct vhost_poll *poll = container_of(wait, struct vhost_poll, wait);
+ struct vhost_virtqueue *vq = container_of(poll, struct vhost_virtqueue,
+ poll);
+
+ if (!(key_to_poll(key) & poll->mask))
+ return 0;
+
+ vq->handle_kick(>poll.work);
+
+ return 0;
+}
+
+void vhost_vdpa_poll_init(struct vhost_dev *dev)
+{
+ struct vhost_virtqueue *vq;
+ struct vhost_poll *poll;
+ int i;
+
+ for (i = 0; i < dev->nvqs; i++) {
+ vq = dev->vqs[i];
+ poll = >poll;
+ if (vq->handle_kick) {
+ init_waitqueue_func_entry(>wait,
+ vhost_vdpa_poll_worker);
+ poll->work.fn = vq->handle_kick;
Why this is needed?
+ }
+
+ }
+}
+
static int vhost_vdpa_open(struct inode *inode, struct file *filep)
{
struct vhost_vdpa *v;
@@ -697,6 +795,8 @@ static int vhost_vdpa_open(struct inode *inode, struct file
*filep)
vhost_dev_init(dev, vqs, nvqs, 0, 0, 0,
vhost_vdpa_process_iotlb_msg);
+ vhost_vdpa_poll_init(dev);
+
dev->iotlb = vhost_iotlb_alloc(0, 0);
if (!dev->iotlb) {
r = -ENOMEM;
So my feeling here is that you want to reuse the infrastructure in
vhost.c as much as possible
If this is true, let's just avoid duplicating the codes. How about
adding something like in vhost_poll_wakeup():
struct vhost_poll *poll = container_of(wait,
Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
Hi Zhu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on vhost/linux-next]
[also build test WARNING on v5.7-rc7 next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vdpa-bypass-waking-up-vhost_woker-for-vdpa-vq-kick/20200526-133819
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.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
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/vhost/vdpa.c:290:6: warning: no previous prototype for
>> 'vhost_vdpa_poll_stop' [-Wmissing-prototypes]
290 | void vhost_vdpa_poll_stop(struct vhost_virtqueue *vq)
| ^~~~
>> drivers/vhost/vdpa.c:295:5: warning: no previous prototype for
>> 'vhost_vdpa_poll_start' [-Wmissing-prototypes]
295 | int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
| ^
>> drivers/vhost/vdpa.c:750:6: warning: no previous prototype for
>> 'vhost_vdpa_poll_init' [-Wmissing-prototypes]
750 | void vhost_vdpa_poll_init(struct vhost_dev *dev)
| ^~~~
vim +/vhost_vdpa_poll_stop +290 drivers/vhost/vdpa.c
276
277 static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user
*argp)
278 {
279 struct vdpa_device *vdpa = v->vdpa;
280 const struct vdpa_config_ops *ops = vdpa->config;
281 u16 num;
282
283 num = ops->get_vq_num_max(vdpa);
284
285 if (copy_to_user(argp, , sizeof(num)))
286 return -EFAULT;
287
288 return 0;
289 }
> 290 void vhost_vdpa_poll_stop(struct vhost_virtqueue *vq)
291 {
292 vhost_poll_stop(>poll);
293 }
294
> 295 int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
296 {
297 struct vhost_poll *poll = >poll;
298 struct file *file = vq->kick;
299 __poll_t mask;
300
301
302 if (poll->wqh)
303 return 0;
304
305 mask = vfs_poll(file, >table);
306 if (mask)
307 vq->handle_kick(>poll.work);
308 if (mask & EPOLLERR) {
309 vhost_poll_stop(poll);
310 return -EINVAL;
311 }
312
313 return 0;
314 }
315
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
