Re: [PATCH RFC 12/14] vhost: add vhost_dev pointer to vhost_work

[Stefano Garzarella <sgarzare@xxxxxxxxxx>]

On Wed, Apr 28, 2021 at 05:37:12PM -0500, Mike Christie wrote:
> The next patch allows a vhost_worker to handle different devices. To
> prepare for that, this patch adds a pointer to the device on the work so
> we can get to the different mms in the vhost_worker thread.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
> drivers/vhost/scsi.c  |  7 ++++---
> drivers/vhost/vhost.c | 24 ++++++++++++++----------
> drivers/vhost/vhost.h | 10 ++++++----
> drivers/vhost/vsock.c |  3 ++-
> 4 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 2f84cf602ab3..0e862503b6a8 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1808,7 +1808,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
> 	if (!vqs)
> 		goto err_vqs;
>
> -	vhost_work_init(&vs->vs_event_work, vhost_scsi_evt_work);
> +	vhost_work_init(&vs->dev, &vs->vs_event_work, vhost_scsi_evt_work);
>
> 	vs->vs_events_nr = 0;
> 	vs->vs_events_missed = false;
> @@ -1823,7 +1823,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
> 		vqs[i] = &svq->vq;
> 		svq->vs = vs;
> 		init_llist_head(&svq->completion_list);
> -		vhost_work_init(&svq->completion_work,
> +		vhost_work_init(&vs->dev, &svq->completion_work,
> 				vhost_scsi_complete_cmd_work);
> 		svq->vq.handle_kick = vhost_scsi_handle_kick;
> 	}
> @@ -2017,7 +2017,8 @@ static int vhost_scsi_port_link(struct se_portal_group *se_tpg,
> 	if (!tmf)
> 		return -ENOMEM;
> 	INIT_LIST_HEAD(&tmf->queue_entry);
> -	vhost_work_init(&tmf->vwork, vhost_scsi_tmf_resp_work);
> +	vhost_work_init(&tpg->vhost_scsi->dev, &tmf->vwork,
> +			 vhost_scsi_tmf_resp_work);

`checkpatch.pl --strict` complains here:

CHECK: Alignment should match open parenthesis
#74: FILE: drivers/vhost/scsi.c:2036:
+	vhost_work_init(&tpg->vhost_scsi->dev, &tmf->vwork,
+			 vhost_scsi_tmf_resp_work);

> 	mutex_lock(&vhost_scsi_mutex);
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index fecdae0d18c7..7ba0c303bb98 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -181,10 +181,12 @@ static int vhost_poll_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync,
> 	return 0;
> }
>
> -void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
> +void vhost_work_init(struct vhost_dev *dev, struct vhost_work *work,
> +		     vhost_work_fn_t fn)
> {
> 	clear_bit(VHOST_WORK_QUEUED, &work->flags);
> 	work->fn = fn;
> +	work->dev = dev;
> }
> EXPORT_SYMBOL_GPL(vhost_work_init);
>
> @@ -200,7 +202,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> 	poll->wqh = NULL;
> 	poll->vq = vq;
>
> -	vhost_work_init(&poll->work, fn);
> +	vhost_work_init(dev, &poll->work, fn);
> }
> EXPORT_SYMBOL_GPL(vhost_poll_init);
>
> @@ -269,12 +271,13 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)
> }
> EXPORT_SYMBOL_GPL(vhost_work_queue);
>
> -static void vhost_work_dev_flush_on(struct vhost_worker *worker)
> +static void vhost_work_dev_flush_on(struct vhost_dev *dev,
> +				    struct vhost_worker *worker)
> {
> 	struct vhost_flush_struct flush;
>
> 	init_completion(&flush.wait_event);
> -	vhost_work_init(&flush.work, vhost_flush_work);
> +	vhost_work_init(dev, &flush.work, vhost_flush_work);
>
> 	vhost_work_queue_on(&flush.work, worker);
> 	wait_for_completion(&flush.wait_event);
> @@ -285,7 +288,7 @@ void vhost_work_dev_flush(struct vhost_dev *dev)
> 	int i;
>
> 	for (i = 0; i < dev->num_workers; i++)
> -		vhost_work_dev_flush_on(dev->workers[i]);
> +		vhost_work_dev_flush_on(dev, dev->workers[i]);
> }
> EXPORT_SYMBOL_GPL(vhost_work_dev_flush);
>
> @@ -305,7 +308,7 @@ EXPORT_SYMBOL_GPL(vhost_has_work);
>
> void vhost_vq_work_flush(struct vhost_virtqueue *vq)
> {
> -	vhost_work_dev_flush_on(vq->worker);
> +	vhost_work_dev_flush_on(vq->dev, vq->worker);
> }
> EXPORT_SYMBOL_GPL(vhost_vq_work_flush);
>
> @@ -572,14 +575,15 @@ static void vhost_attach_cgroups_work(struct vhost_work *work)
> 	s->ret = cgroup_attach_task_all(s->owner, current);
> }
>
> -static int vhost_attach_cgroups_on(struct vhost_worker *worker)
> +static int vhost_attach_cgroups_on(struct vhost_dev *dev,
> +				   struct vhost_worker *worker)
> {
> 	struct vhost_attach_cgroups_struct attach;
>
> 	attach.owner = current;
> -	vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> +	vhost_work_init(dev, &attach.work, vhost_attach_cgroups_work);
> 	vhost_work_queue_on(&attach.work, worker);
> -	vhost_work_dev_flush_on(worker);
> +	vhost_work_dev_flush_on(dev, worker);
> 	return attach.ret;
> }
>
> @@ -673,7 +677,7 @@ static struct vhost_worker 
> *vhost_worker_create(struct vhost_dev *dev)
> 	worker->task = task;
> 	wake_up_process(task); /* avoid contributing to loadavg */
>
> -	ret = vhost_attach_cgroups_on(worker);
> +	ret = vhost_attach_cgroups_on(dev, worker);
> 	if (ret)
> 		goto stop_worker;
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 64dc00337389..051dea4e3ab6 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -21,9 +21,10 @@ typedef void (*vhost_work_fn_t)(struct vhost_work *work);
>
> #define VHOST_WORK_QUEUED 1
> struct vhost_work {
> -	struct llist_node	  node;
> -	vhost_work_fn_t		  fn;
> -	unsigned long		  flags;
> +	struct llist_node	node;
> +	vhost_work_fn_t		fn;
> +	unsigned long		flags;

Maybe we should move these changes in another patch since they are not 
related.

Thanks,
Stefano

> +	struct vhost_dev	*dev;
> };
>
> struct vhost_worker {
> @@ -47,7 +48,8 @@ struct vhost_poll {
> 	struct vhost_virtqueue	*vq;
> };
>
> -void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
> +void vhost_work_init(struct vhost_dev *dev, struct vhost_work *work,
> +		     vhost_work_fn_t fn);
> void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> bool vhost_has_work(struct vhost_dev *dev);
> void vhost_vq_work_flush(struct vhost_virtqueue *vq);
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index f954f4d29c95..302415b6460b 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -648,7 +648,8 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
> 	file->private_data = vsock;
> 	spin_lock_init(&vsock->send_pkt_list_lock);
> 	INIT_LIST_HEAD(&vsock->send_pkt_list);
> -	vhost_work_init(&vsock->send_pkt_work, vhost_transport_send_pkt_work);
> +	vhost_work_init(&vsock->dev, &vsock->send_pkt_work,
> +			vhost_transport_send_pkt_work);
> 	return 0;
>
> out:
> --
> 2.25.1

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