Re: [PATCH 1/4] vdpa: Avoid taking cf_mutex lock on get status

[Si-Wei Liu <si-wei.liu@xxxxxxxxxx>]

On 1/11/2022 10:33 AM, Eli Cohen wrote:
> Avoid the wrapper holding cf_mutex since it is not protecting anything.
> To avoid confusion and unnecessary overhead incurred by it, remove.
>
> Fixes: f489f27bc0ab ("vdpa: Sync calls set/get config/status with cf_mutex")
> Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
Reviewed-by: Si-Wei Liu<si-wei.liu@xxxxxxxxxx>
> ---
>   drivers/vdpa/vdpa.c  | 11 -----------
>   drivers/vhost/vdpa.c |  5 +++--
>   include/linux/vdpa.h |  1 -
>   3 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 4380367d00b5..9846c9de4bfa 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -21,17 +21,6 @@ static LIST_HEAD(mdev_head);
>   static DEFINE_MUTEX(vdpa_dev_mutex);
>   static DEFINE_IDA(vdpa_index_ida);
>   
> -u8 vdpa_get_status(struct vdpa_device *vdev)
> -{
> -	u8 status;
> -
> -	mutex_lock(&vdev->cf_mutex);
> -	status = vdev->config->get_status(vdev);
> -	mutex_unlock(&vdev->cf_mutex);
> -	return status;
> -}
> -EXPORT_SYMBOL(vdpa_get_status);
> -
>   void vdpa_set_status(struct vdpa_device *vdev, u8 status)
>   {
>   	mutex_lock(&vdev->cf_mutex);
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 6e7edaf2472b..0ed6cbadb52d 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -142,9 +142,10 @@ static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
>   static long vhost_vdpa_get_status(struct vhost_vdpa *v, u8 __user *statusp)
>   {
>   	struct vdpa_device *vdpa = v->vdpa;
> +	const struct vdpa_config_ops *ops = vdpa->config;
>   	u8 status;
>   
> -	status = vdpa_get_status(vdpa);
> +	status = ops->get_status(vdpa);
>   
>   	if (copy_to_user(statusp, &status, sizeof(status)))
>   		return -EFAULT;
> @@ -163,7 +164,7 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
>   	if (copy_from_user(&status, statusp, sizeof(status)))
>   		return -EFAULT;
>   
> -	status_old = vdpa_get_status(vdpa);
> +	status_old = ops->get_status(vdpa);
>   
>   	/*
>   	 * Userspace shouldn't remove status bits unless reset the
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index a6047fd6cf12..2de442ececae 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -421,7 +421,6 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
>   		     void *buf, unsigned int len);
>   void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
>   		     const void *buf, unsigned int length);
> -u8 vdpa_get_status(struct vdpa_device *vdev);
>   void vdpa_set_status(struct vdpa_device *vdev, u8 status);
>   
>   /**

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization