diff mbox series

vdpa: bypass waking up vhost_woker for vdpa vq kick

Message ID 1590471145-4436-1-git-send-email-lingshan.zhu@intel.com (mailing list archive)
State New, archived
Headers show
Series vdpa: bypass waking up vhost_woker for vdpa vq kick | expand

Commit Message

Zhu, Lingshan May 26, 2020, 5:32 a.m. UTC 
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.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
Suggested-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vdpa.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

Comments

kernel test robot May 26, 2020, 10:53 p.m. UTC | #1 
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 <lkp@intel.com>

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, &num, 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(&vq->poll);
   293	}
   294	
 > 295	int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
   296	{
   297		struct vhost_poll *poll = &vq->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, &poll->table);
   306		if (mask)
   307			vq->handle_kick(&vq->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-all@lists.01.org
Jason Wang May 28, 2020, 10:06 a.m. UTC | #2 
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 <lingshan.zhu@intel.com>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> ---
>   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(&vq->poll);
> +}
> +
> +int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
> +{
> +	struct vhost_poll *poll = &vq->poll;
> +	struct file *file = vq->kick;
> +	__poll_t mask;
> +
> +
> +	if (poll->wqh)
> +		return 0;
> +
> +	mask = vfs_poll(file, &poll->table);
> +	if (mask)
> +		vq->handle_kick(&vq->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(&f, 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(&vq->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 = &vq->poll;
> +		if (vq->handle_kick) {
> +			init_waitqueue_func_entry(&poll->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, struct vhost_poll, wait);
     struct vhost_work *work = &poll->work;

     if (!(key_to_poll(key) & poll->mask))
         return 0;

     if (!poll->dev->use_worker)
         work->fn(work);
     else
         vhost_poll_queue(poll);


Then modify vhost_dev_init() to set use_worker (all true except for vdpa)?


Thanks
Dan Carpenter June 2, 2020, 9:42 a.m. UTC | #3 
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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

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(&f, 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-all@lists.01.org
Jason Wang June 2, 2020, 10:16 a.m. UTC | #4 
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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> 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(&f, 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-all@lists.01.org
Jason Wang June 2, 2020, 11:10 a.m. UTC | #5 
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 <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> 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(&f, 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
diff mbox series

Patch

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(&vq->poll);
+}
+
+int vhost_vdpa_poll_start(struct vhost_virtqueue *vq)
+{
+	struct vhost_poll *poll = &vq->poll;
+	struct file *file = vq->kick;
+	__poll_t mask;
+
+
+	if (poll->wqh)
+		return 0;
+
+	mask = vfs_poll(file, &poll->table);
+	if (mask)
+		vq->handle_kick(&vq->poll.work);
+	if (mask & EPOLLERR) {
+		vhost_poll_stop(poll);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+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(&f, 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(&vq->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 = &vq->poll;
+		if (vq->handle_kick) {
+			init_waitqueue_func_entry(&poll->wait,
+						  vhost_vdpa_poll_worker);
+			poll->work.fn = vq->handle_kick;
+		}
+
+	}
+}
+
 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;