Received-SPF: pass (google.com: domain of linux-kernel+bounces-148233-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 17 Apr 2024 05:29:37 -0400
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Cindy Lu <lulu@redhat.com>
Cc: jasowang@redhat.com, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v5 3/5] vduse: Add function to get/free the pages for
 reconnection
Message-ID: <20240417052723-mutt-send-email-mst@kernel.org>
References: <20240412133017.483407-1-lulu@redhat.com>
 <20240412133017.483407-4-lulu@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240412133017.483407-4-lulu@redhat.com>

On Fri, Apr 12, 2024 at 09:28:23PM +0800, Cindy Lu wrote:
> Add the function vduse_alloc_reconnnect_info_mem
> and vduse_alloc_reconnnect_info_mem
> These functions allow vduse to allocate and free memory for reconnection
> information. The amount of memory allocated is vq_num pages.
> Each VQS will map its own page where the reconnection information will be saved
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 40 ++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index ef3c9681941e..2da659d5f4a8 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -65,6 +65,7 @@ struct vduse_virtqueue {
>  	int irq_effective_cpu;
>  	struct cpumask irq_affinity;
>  	struct kobject kobj;
> +	unsigned long vdpa_reconnect_vaddr;
>  };
>  
>  struct vduse_dev;
> @@ -1105,6 +1106,38 @@ static void vduse_vq_update_effective_cpu(struct vduse_virtqueue *vq)
>  
>  	vq->irq_effective_cpu = curr_cpu;
>  }
> +static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> +	unsigned long vaddr = 0;
> +	struct vduse_virtqueue *vq;
> +
> +	for (int i = 0; i < dev->vq_num; i++) {
> +		/*page 0~ vq_num save the reconnect info for vq*/
> +		vq = dev->vqs[i];
> +		vaddr = get_zeroed_page(GFP_KERNEL);


I don't get why you insist on stealing kernel memory for something
that is just used by userspace to store data for its own use.
Userspace does not lack ways to persist data, for example,
create a regular file anywhere in the filesystem.


> +		if (vaddr == 0)
> +			return -ENOMEM;
> +
> +		vq->vdpa_reconnect_vaddr = vaddr;
> +	}
> +
> +	return 0;
> +}
> +
> +static int vduse_free_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> +	struct vduse_virtqueue *vq;
> +
> +	for (int i = 0; i < dev->vq_num; i++) {
> +		vq = dev->vqs[i];
> +
> +		if (vq->vdpa_reconnect_vaddr)
> +			free_page(vq->vdpa_reconnect_vaddr);
> +		vq->vdpa_reconnect_vaddr = 0;
> +	}
> +
> +	return 0;
> +}
>  
>  static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>  			    unsigned long arg)
> @@ -1672,6 +1705,8 @@ static int vduse_destroy_dev(char *name)
>  		mutex_unlock(&dev->lock);
>  		return -EBUSY;
>  	}
> +	vduse_free_reconnnect_info_mem(dev);
> +
>  	dev->connected = true;
>  	mutex_unlock(&dev->lock);
>  
> @@ -1855,12 +1890,17 @@ static int vduse_create_dev(struct vduse_dev_config *config,
>  	ret = vduse_dev_init_vqs(dev, config->vq_align, config->vq_num);
>  	if (ret)
>  		goto err_vqs;
> +	ret = vduse_alloc_reconnnect_info_mem(dev);
> +	if (ret < 0)
> +		goto err_mem;
>  
>  	__module_get(THIS_MODULE);
>  
>  	return 0;
>  err_vqs:
>  	device_destroy(&vduse_class, MKDEV(MAJOR(vduse_major), dev->minor));
> +err_mem:
> +	vduse_free_reconnnect_info_mem(dev);
>  err_dev:
>  	idr_remove(&vduse_idr, dev->minor);
>  err_idr:
> -- 
> 2.43.0