Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1680567 > unrolled thread

[PATCH v6 00/18] introduce the Xen PV Calls backend

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 00/18] introduce the Xen PV Calls backend Stefano Stabellini <stefano@aporeto.com> - 2017-07-03 23:10 +0200
    [PATCH v6 02/18] xen/pvcalls: introduce the pvcalls xenbus backend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
    [PATCH v6 16/18] xen/pvcalls: implement read Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
      Re: [PATCH v6 16/18] xen/pvcalls: implement read Juergen Gross <jgross@suse.com> - 2017-07-04 09:40 +0200
    [PATCH v6 08/18] xen/pvcalls: implement connect command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
      Re: [PATCH v6 08/18] xen/pvcalls: implement connect command Juergen Gross <jgross@suse.com> - 2017-07-04 09:20 +0200
        Re: [PATCH v6 08/18] xen/pvcalls: implement connect command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 23:20 +0200
    [PATCH v6 07/18] xen/pvcalls: implement socket command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
    [PATCH v6 05/18] xen/pvcalls: connect to a frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
      Re: [PATCH v6 05/18] xen/pvcalls: connect to a frontend Juergen Gross <jgross@suse.com> - 2017-07-04 09:00 +0200
        Re: [PATCH v6 05/18] xen/pvcalls: connect to a frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 22:40 +0200
    [PATCH v6 13/18] xen/pvcalls: implement release command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:10 +0200
      Re: [PATCH v6 13/18] xen/pvcalls: implement release command Juergen Gross <jgross@suse.com> - 2017-07-04 09:40 +0200
    [PATCH v6 06/18] xen/pvcalls: handle commands from the frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
      Re: [PATCH v6 06/18] xen/pvcalls: handle commands from the frontend Juergen Gross <jgross@suse.com> - 2017-07-04 09:00 +0200
        Re: [PATCH v6 06/18] xen/pvcalls: handle commands from the  frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 22:50 +0200
    [PATCH v6 01/18] xen: introduce the pvcalls interface header Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
      [PATCH v6 11/18] xen/pvcalls: implement accept command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 11/18] xen/pvcalls: implement accept command Juergen Gross <jgross@suse.com> - 2017-07-04 09:30 +0200
      [PATCH v6 09/18] xen/pvcalls: implement bind command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 09/18] xen/pvcalls: implement bind command Juergen Gross <jgross@suse.com> - 2017-07-04 09:30 +0200
          Re: [PATCH v6 09/18] xen/pvcalls: implement bind command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 23:20 +0200
      [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit Juergen Gross <jgross@suse.com> - 2017-07-04 09:40 +0200
          Re: [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 23:30 +0200
      [PATCH v6 03/18] xen/pvcalls: initialize the module and register the xenbus backend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
      [PATCH v6 12/18] xen/pvcalls: implement poll command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 12/18] xen/pvcalls: implement poll command Juergen Gross <jgross@suse.com> - 2017-07-04 09:30 +0200
      [PATCH v6 10/18] xen/pvcalls: implement listen command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
      [PATCH v6 04/18] xen/pvcalls: xenbus state handling Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 04/18] xen/pvcalls: xenbus state handling Juergen Gross <jgross@suse.com> - 2017-07-04 10:10 +0200
          Re: [PATCH v6 04/18] xen/pvcalls: xenbus state handling Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 22:30 +0200
      [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions Stefano Stabellini <sstabellini@kernel.org> - 2017-07-03 23:20 +0200
        Re: [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions Juergen Gross <jgross@suse.com> - 2017-07-04 09:50 +0200
          Re: [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions Stefano Stabellini <sstabellini@kernel.org> - 2017-07-05 23:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1680718 — Re: [PATCH v6 09/18] xen/pvcalls: implement bind command

On 03/07/17 23:08, Stefano Stabellini wrote:
> Allocate a socket. Track the allocated passive sockets with a new data
> structure named sockpass_mapping. It contains an unbound workqueue to
> schedule delayed work for the accept and poll commands. It also has a
> reqcopy field to be used to store a copy of a request for delayed work.
> Reads/writes to it are protected by a lock (the "copy_lock" spinlock).
> Initialize the workqueue in pvcalls_back_bind.
> 
> Implement the bind command with inet_bind.
> 
> The pass_sk_data_ready event handler will be added later.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: boris.ostrovsky@oracle.com
> CC: jgross@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 1bc2620..dae91fb 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -78,6 +78,18 @@ struct sock_mapping {
>  	struct pvcalls_ioworker ioworker;
>  };
>  
> +struct sockpass_mapping {
> +	struct list_head list;
> +	struct pvcalls_fedata *fedata;
> +	struct socket *sock;
> +	uint64_t id;
> +	struct xen_pvcalls_request reqcopy;
> +	spinlock_t copy_lock;
> +	struct workqueue_struct *wq;
> +	struct work_struct register_work;
> +	void (*saved_data_ready)(struct sock *sk);
> +};
> +
>  static irqreturn_t pvcalls_back_conn_event(int irq, void *sock_map);
>  static int pvcalls_back_release_active(struct xenbus_device *dev,
>  				       struct pvcalls_fedata *fedata,
> @@ -263,9 +275,84 @@ static int pvcalls_back_release(struct xenbus_device *dev,
>  	return 0;
>  }
>  
> +static void __pvcalls_back_accept(struct work_struct *work)
> +{
> +}
> +
> +static void pvcalls_pass_sk_data_ready(struct sock *sock)
> +{
> +}
> +
>  static int pvcalls_back_bind(struct xenbus_device *dev,
>  			     struct xen_pvcalls_request *req)
>  {
> +	struct pvcalls_fedata *fedata;
> +	int ret, err;
> +	struct socket *sock;

Get rid of sock, ...

> +	struct sockpass_mapping *map;
> +	struct xen_pvcalls_response *rsp;
> +
> +	fedata = dev_get_drvdata(&dev->dev);
> +
> +	map = kzalloc(sizeof(*map), GFP_KERNEL);
> +	if (map == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	INIT_WORK(&map->register_work, __pvcalls_back_accept);
> +	spin_lock_init(&map->copy_lock);
> +	map->wq = alloc_workqueue("pvcalls_wq", WQ_UNBOUND, 1);
> +	if (!map->wq) {
> +		ret = -ENOMEM;
> +		kfree(map);

Move kfree(map) to the exit path, ...

> +		goto out;
> +	}
> +
> +	ret = sock_create(AF_INET, SOCK_STREAM, 0, &sock);

use &map->sock here, ...

> +	if (ret < 0) {
> +		destroy_workqueue(map->wq);

move destory_workqueue() to the exit path, ...

> +		kfree(map);
> +		goto out;
> +	}
> +
> +	ret = inet_bind(sock, (struct sockaddr *)&req->u.bind.addr,
> +			req->u.bind.len);
> +	if (ret < 0) {
> +		sock_release(sock);

and sock_release, too, ...

> +		destroy_workqueue(map->wq);
> +		kfree(map);
> +		goto out;
> +	}
> +
> +	map->fedata = fedata;
> +	map->sock = sock;
> +	map->id = req->u.bind.id;
> +
> +	down(&fedata->socket_lock);
> +	err = radix_tree_insert(&fedata->socketpass_mappings, map->id,
> +				map);

User ret instead of err?

> +	up(&fedata->socket_lock);
> +	if (err) {
> +		ret = err;
> +		sock_release(sock);
> +		destroy_workqueue(map->wq);
> +		kfree(map);
> +		goto out;
> +	}
> +
> +	write_lock_bh(&sock->sk->sk_callback_lock);
> +	map->saved_data_ready = sock->sk->sk_data_ready;
> +	sock->sk->sk_user_data = map;
> +	sock->sk->sk_data_ready = pvcalls_pass_sk_data_ready;
> +	write_unlock_bh(&sock->sk->sk_callback_lock);
> +
> +out:
> +	rsp = RING_GET_RESPONSE(&fedata->ring, fedata->ring.rsp_prod_pvt++);
> +	rsp->req_id = req->req_id;
> +	rsp->cmd = req->cmd;
> +	rsp->u.bind.id = req->u.bind.id;
> +	rsp->ret = ret;

... have a common error exit handling:
+	if (ret) {
+		if (map && map->sock)
+			sock_release(map->sock);
+		if (map && map->wq)
+			destroy_workqueue(map->wq);
+		kfree(map);
+	}


Juergen

[toc] | [prev] | [next] | [standalone]

#1681814 — Re: [PATCH v6 09/18] xen/pvcalls: implement bind command

On Tue, 4 Jul 2017, Juergen Gross wrote:
> On 03/07/17 23:08, Stefano Stabellini wrote:
> > Allocate a socket. Track the allocated passive sockets with a new data
> > structure named sockpass_mapping. It contains an unbound workqueue to
> > schedule delayed work for the accept and poll commands. It also has a
> > reqcopy field to be used to store a copy of a request for delayed work.
> > Reads/writes to it are protected by a lock (the "copy_lock" spinlock).
> > Initialize the workqueue in pvcalls_back_bind.
> > 
> > Implement the bind command with inet_bind.
> > 
> > The pass_sk_data_ready event handler will be added later.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: boris.ostrovsky@oracle.com
> > CC: jgross@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 87 insertions(+)
> > 
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 1bc2620..dae91fb 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -78,6 +78,18 @@ struct sock_mapping {
> >  	struct pvcalls_ioworker ioworker;
> >  };
> >  
> > +struct sockpass_mapping {
> > +	struct list_head list;
> > +	struct pvcalls_fedata *fedata;
> > +	struct socket *sock;
> > +	uint64_t id;
> > +	struct xen_pvcalls_request reqcopy;
> > +	spinlock_t copy_lock;
> > +	struct workqueue_struct *wq;
> > +	struct work_struct register_work;
> > +	void (*saved_data_ready)(struct sock *sk);
> > +};
> > +
> >  static irqreturn_t pvcalls_back_conn_event(int irq, void *sock_map);
> >  static int pvcalls_back_release_active(struct xenbus_device *dev,
> >  				       struct pvcalls_fedata *fedata,
> > @@ -263,9 +275,84 @@ static int pvcalls_back_release(struct xenbus_device *dev,
> >  	return 0;
> >  }
> >  
> > +static void __pvcalls_back_accept(struct work_struct *work)
> > +{
> > +}
> > +
> > +static void pvcalls_pass_sk_data_ready(struct sock *sock)
> > +{
> > +}
> > +
> >  static int pvcalls_back_bind(struct xenbus_device *dev,
> >  			     struct xen_pvcalls_request *req)
> >  {
> > +	struct pvcalls_fedata *fedata;
> > +	int ret, err;
> > +	struct socket *sock;
> 
> Get rid of sock, ...

OK


> > +	struct sockpass_mapping *map;
> > +	struct xen_pvcalls_response *rsp;
> > +
> > +	fedata = dev_get_drvdata(&dev->dev);
> > +
> > +	map = kzalloc(sizeof(*map), GFP_KERNEL);
> > +	if (map == NULL) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	INIT_WORK(&map->register_work, __pvcalls_back_accept);
> > +	spin_lock_init(&map->copy_lock);
> > +	map->wq = alloc_workqueue("pvcalls_wq", WQ_UNBOUND, 1);
> > +	if (!map->wq) {
> > +		ret = -ENOMEM;
> > +		kfree(map);
> 
> Move kfree(map) to the exit path, ...

OK


> > +		goto out;
> > +	}
> > +
> > +	ret = sock_create(AF_INET, SOCK_STREAM, 0, &sock);
> 
> use &map->sock here, ...

OK


> > +	if (ret < 0) {
> > +		destroy_workqueue(map->wq);
> 
> move destory_workqueue() to the exit path, ...

OK


> > +		kfree(map);
> > +		goto out;
> > +	}
> > +
> > +	ret = inet_bind(sock, (struct sockaddr *)&req->u.bind.addr,
> > +			req->u.bind.len);
> > +	if (ret < 0) {
> > +		sock_release(sock);
> 
> and sock_release, too, ...

OK


> > +		destroy_workqueue(map->wq);
> > +		kfree(map);
> > +		goto out;
> > +	}
> > +
> > +	map->fedata = fedata;
> > +	map->sock = sock;
> > +	map->id = req->u.bind.id;
> > +
> > +	down(&fedata->socket_lock);
> > +	err = radix_tree_insert(&fedata->socketpass_mappings, map->id,
> > +				map);
> 
> User ret instead of err?

OK


> > +	up(&fedata->socket_lock);
> > +	if (err) {
> > +		ret = err;
> > +		sock_release(sock);
> > +		destroy_workqueue(map->wq);
> > +		kfree(map);
> > +		goto out;
> > +	}
> > +
> > +	write_lock_bh(&sock->sk->sk_callback_lock);
> > +	map->saved_data_ready = sock->sk->sk_data_ready;
> > +	sock->sk->sk_user_data = map;
> > +	sock->sk->sk_data_ready = pvcalls_pass_sk_data_ready;
> > +	write_unlock_bh(&sock->sk->sk_callback_lock);
> > +
> > +out:
> > +	rsp = RING_GET_RESPONSE(&fedata->ring, fedata->ring.rsp_prod_pvt++);
> > +	rsp->req_id = req->req_id;
> > +	rsp->cmd = req->cmd;
> > +	rsp->u.bind.id = req->u.bind.id;
> > +	rsp->ret = ret;
> 
> ... have a common error exit handling:
> +	if (ret) {
> +		if (map && map->sock)
> +			sock_release(map->sock);
> +		if (map && map->wq)
> +			destroy_workqueue(map->wq);
> +		kfree(map);
> +	}
 
All good suggestions, I'll make all changes.

[toc] | [prev] | [next] | [standalone]

#1680583 — [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit

Implement backend_disconnect. Call pvcalls_back_release_active on active
sockets and pvcalls_back_release_passive on passive sockets.

Implement module_exit by calling backend_disconnect on frontend
connections.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 9f4247f..71a42fc 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -807,6 +807,42 @@ static int backend_connect(struct xenbus_device *dev)
 
 static int backend_disconnect(struct xenbus_device *dev)
 {
+	struct pvcalls_fedata *fedata;
+	struct sock_mapping *map, *n;
+	struct sockpass_mapping *mappass;
+	struct radix_tree_iter iter;
+	void **slot;
+
+
+	fedata = dev_get_drvdata(&dev->dev);
+
+	down(&fedata->socket_lock);
+	list_for_each_entry_safe(map, n, &fedata->socket_mappings, list) {
+		list_del(&map->list);
+		pvcalls_back_release_active(dev, fedata, map);
+	}
+
+	radix_tree_for_each_slot(slot, &fedata->socketpass_mappings, &iter, 0) {
+		mappass = radix_tree_deref_slot(slot);
+		if (!mappass)
+			continue;
+		if (radix_tree_exception(mappass)) {
+			if (radix_tree_deref_retry(mappass))
+				slot = radix_tree_iter_retry(&iter);
+		} else {
+			radix_tree_delete(&fedata->socketpass_mappings, mappass->id);
+			pvcalls_back_release_passive(dev, fedata, mappass);
+		}
+	}
+	up(&fedata->socket_lock);
+
+	xenbus_unmap_ring_vfree(dev, fedata->sring);
+	unbind_from_irqhandler(fedata->irq, dev);
+
+	list_del(&fedata->list);
+	kfree(fedata);
+	dev_set_drvdata(&dev->dev, NULL);
+
 	return 0;
 }
 
@@ -1000,3 +1036,19 @@ static int __init pvcalls_back_init(void)
 	return 0;
 }
 module_init(pvcalls_back_init);
+
+static void __exit pvcalls_back_fin(void)
+{
+	struct pvcalls_fedata *fedata, *nfedata;
+
+	down(&pvcalls_back_global.frontends_lock);
+	list_for_each_entry_safe(fedata, nfedata, &pvcalls_back_global.frontends,
+				 list) {
+		backend_disconnect(fedata->dev);
+	}
+	up(&pvcalls_back_global.frontends_lock);
+
+	xenbus_unregister_driver(&pvcalls_back_driver);
+}
+
+module_exit(pvcalls_back_fin);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680725 — Re: [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit

On 03/07/17 23:08, Stefano Stabellini wrote:
> Implement backend_disconnect. Call pvcalls_back_release_active on active
> sockets and pvcalls_back_release_passive on passive sockets.
> 
> Implement module_exit by calling backend_disconnect on frontend
> connections.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: boris.ostrovsky@oracle.com
> CC: jgross@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 9f4247f..71a42fc 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -807,6 +807,42 @@ static int backend_connect(struct xenbus_device *dev)
>  
>  static int backend_disconnect(struct xenbus_device *dev)
>  {
> +	struct pvcalls_fedata *fedata;
> +	struct sock_mapping *map, *n;
> +	struct sockpass_mapping *mappass;
> +	struct radix_tree_iter iter;
> +	void **slot;
> +
> +
> +	fedata = dev_get_drvdata(&dev->dev);
> +
> +	down(&fedata->socket_lock);
> +	list_for_each_entry_safe(map, n, &fedata->socket_mappings, list) {
> +		list_del(&map->list);
> +		pvcalls_back_release_active(dev, fedata, map);
> +	}
> +
> +	radix_tree_for_each_slot(slot, &fedata->socketpass_mappings, &iter, 0) {
> +		mappass = radix_tree_deref_slot(slot);
> +		if (!mappass)
> +			continue;
> +		if (radix_tree_exception(mappass)) {
> +			if (radix_tree_deref_retry(mappass))
> +				slot = radix_tree_iter_retry(&iter);
> +		} else {
> +			radix_tree_delete(&fedata->socketpass_mappings, mappass->id);
> +			pvcalls_back_release_passive(dev, fedata, mappass);
> +		}
> +	}
> +	up(&fedata->socket_lock);
> +
> +	xenbus_unmap_ring_vfree(dev, fedata->sring);
> +	unbind_from_irqhandler(fedata->irq, dev);

Swap above two lines to avoid irq being handled after releasing
ring?


Juergen

[toc] | [prev] | [next] | [standalone]

#1681823 — Re: [PATCH v6 14/18] xen/pvcalls: disconnect and module_exit

On Tue, 4 Jul 2017, Juergen Gross wrote:
> On 03/07/17 23:08, Stefano Stabellini wrote:
> > Implement backend_disconnect. Call pvcalls_back_release_active on active
> > sockets and pvcalls_back_release_passive on passive sockets.
> > 
> > Implement module_exit by calling backend_disconnect on frontend
> > connections.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: boris.ostrovsky@oracle.com
> > CC: jgross@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> > 
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 9f4247f..71a42fc 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -807,6 +807,42 @@ static int backend_connect(struct xenbus_device *dev)
> >  
> >  static int backend_disconnect(struct xenbus_device *dev)
> >  {
> > +	struct pvcalls_fedata *fedata;
> > +	struct sock_mapping *map, *n;
> > +	struct sockpass_mapping *mappass;
> > +	struct radix_tree_iter iter;
> > +	void **slot;
> > +
> > +
> > +	fedata = dev_get_drvdata(&dev->dev);
> > +
> > +	down(&fedata->socket_lock);
> > +	list_for_each_entry_safe(map, n, &fedata->socket_mappings, list) {
> > +		list_del(&map->list);
> > +		pvcalls_back_release_active(dev, fedata, map);
> > +	}
> > +
> > +	radix_tree_for_each_slot(slot, &fedata->socketpass_mappings, &iter, 0) {
> > +		mappass = radix_tree_deref_slot(slot);
> > +		if (!mappass)
> > +			continue;
> > +		if (radix_tree_exception(mappass)) {
> > +			if (radix_tree_deref_retry(mappass))
> > +				slot = radix_tree_iter_retry(&iter);
> > +		} else {
> > +			radix_tree_delete(&fedata->socketpass_mappings, mappass->id);
> > +			pvcalls_back_release_passive(dev, fedata, mappass);
> > +		}
> > +	}
> > +	up(&fedata->socket_lock);
> > +
> > +	xenbus_unmap_ring_vfree(dev, fedata->sring);
> > +	unbind_from_irqhandler(fedata->irq, dev);
> 
> Swap above two lines to avoid irq being handled after releasing
> ring?

Will do

[toc] | [prev] | [next] | [standalone]

#1680585 — [PATCH v6 03/18] xen/pvcalls: initialize the module and register the xenbus backend

Keep a list of connected frontends. Use a semaphore to protect list
accesses.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index f3d0daa..9044cf2 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -25,6 +25,11 @@
 #include <xen/xenbus.h>
 #include <xen/interface/io/pvcalls.h>
 
+struct pvcalls_back_global {
+	struct list_head frontends;
+	struct semaphore frontends_lock;
+} pvcalls_back_global;
+
 static int pvcalls_back_probe(struct xenbus_device *dev,
 			      const struct xenbus_device_id *id)
 {
@@ -59,3 +64,20 @@ static int pvcalls_back_uevent(struct xenbus_device *xdev,
 	.uevent = pvcalls_back_uevent,
 	.otherend_changed = pvcalls_back_changed,
 };
+
+static int __init pvcalls_back_init(void)
+{
+	int ret;
+
+	if (!xen_domain())
+		return -ENODEV;
+
+	ret = xenbus_register_backend(&pvcalls_back_driver);
+	if (ret < 0)
+		return ret;
+
+	sema_init(&pvcalls_back_global.frontends_lock, 1);
+	INIT_LIST_HEAD(&pvcalls_back_global.frontends);
+	return 0;
+}
+module_init(pvcalls_back_init);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680586 — [PATCH v6 12/18] xen/pvcalls: implement poll command

Implement poll on passive sockets by requesting a delayed response with
mappass->reqcopy, and reply back when there is data on the passive
socket.

Poll on active socket is unimplemented as by the spec, as the frontend
should just wait for events and check the indexes on the indexes page.

Only support one outstanding poll (or accept) request for every passive
socket at any given time.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 73 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 75b7b9a9..dba7bbf 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -350,11 +350,33 @@ static void __pvcalls_back_accept(struct work_struct *work)
 static void pvcalls_pass_sk_data_ready(struct sock *sock)
 {
 	struct sockpass_mapping *mappass = sock->sk_user_data;
+	struct pvcalls_fedata *fedata;
+	struct xen_pvcalls_response *rsp;
+	unsigned long flags;
+	int notify;
 
 	if (mappass == NULL)
 		return;
 
-	queue_work(mappass->wq, &mappass->register_work);
+	fedata = mappass->fedata;
+	spin_lock_irqsave(&mappass->copy_lock, flags);
+	if (mappass->reqcopy.cmd == PVCALLS_POLL) {
+		rsp = RING_GET_RESPONSE(&fedata->ring, fedata->ring.rsp_prod_pvt++);
+		rsp->req_id = mappass->reqcopy.req_id;
+		rsp->u.poll.id = mappass->reqcopy.u.poll.id;
+		rsp->cmd = mappass->reqcopy.cmd;
+		rsp->ret = 0;
+
+		mappass->reqcopy.cmd = 0;
+		spin_unlock_irqrestore(&mappass->copy_lock, flags);
+
+		RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&fedata->ring, notify);
+		if (notify)
+			notify_remote_via_irq(mappass->fedata->irq);
+	} else {
+		spin_unlock_irqrestore(&mappass->copy_lock, flags);
+		queue_work(mappass->wq, &mappass->register_work);
+	}
 }
 
 static int pvcalls_back_bind(struct xenbus_device *dev,
@@ -505,6 +527,55 @@ static int pvcalls_back_accept(struct xenbus_device *dev,
 static int pvcalls_back_poll(struct xenbus_device *dev,
 			     struct xen_pvcalls_request *req)
 {
+	struct pvcalls_fedata *fedata;
+	struct sockpass_mapping *mappass;
+	struct xen_pvcalls_response *rsp;
+	struct inet_connection_sock *icsk;
+	struct request_sock_queue *queue;
+	unsigned long flags;
+	int ret;
+	bool data;
+
+	fedata = dev_get_drvdata(&dev->dev);
+
+	down(&fedata->socket_lock);
+	mappass = radix_tree_lookup(&fedata->socketpass_mappings, req->u.poll.id);
+	up(&fedata->socket_lock);
+	if (mappass == NULL)
+		return -EINVAL;
+
+	/*
+	 * Limitation of the current implementation: only support one
+	 * concurrent accept or poll call on one socket.
+	 */
+	spin_lock_irqsave(&mappass->copy_lock, flags);
+	if (mappass->reqcopy.cmd != 0) {
+		ret = -EINTR;
+		goto out;
+	}
+
+	mappass->reqcopy = *req;
+	icsk = inet_csk(mappass->sock->sk);
+	queue = &icsk->icsk_accept_queue;
+	data = queue->rskq_accept_head != NULL;
+	if (data) {
+		mappass->reqcopy.cmd = 0;
+		ret = 0;
+		goto out;
+	}
+	spin_unlock_irqrestore(&mappass->copy_lock, flags);
+
+	/* Tell the caller we don't need to send back a notification yet */
+	return -1;
+
+out:
+	spin_unlock_irqrestore(&mappass->copy_lock, flags);
+
+	rsp = RING_GET_RESPONSE(&fedata->ring, fedata->ring.rsp_prod_pvt++);
+	rsp->req_id = req->req_id;
+	rsp->cmd = req->cmd;
+	rsp->u.poll.id = req->u.poll.id;
+	rsp->ret = ret;
 	return 0;
 }
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680721 — Re: [PATCH v6 12/18] xen/pvcalls: implement poll command

On 03/07/17 23:08, Stefano Stabellini wrote:
> Implement poll on passive sockets by requesting a delayed response with
> mappass->reqcopy, and reply back when there is data on the passive
> socket.
> 
> Poll on active socket is unimplemented as by the spec, as the frontend
> should just wait for events and check the indexes on the indexes page.
> 
> Only support one outstanding poll (or accept) request for every passive
> socket at any given time.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Thanks,

Juergen

[toc] | [prev] | [next] | [standalone]

#1680589 — [PATCH v6 10/18] xen/pvcalls: implement listen command

Call inet_listen to implement the listen command.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index dae91fb..689b84f 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -359,6 +359,27 @@ static int pvcalls_back_bind(struct xenbus_device *dev,
 static int pvcalls_back_listen(struct xenbus_device *dev,
 			       struct xen_pvcalls_request *req)
 {
+	struct pvcalls_fedata *fedata;
+	int ret = -EINVAL;
+	struct sockpass_mapping *map;
+	struct xen_pvcalls_response *rsp;
+
+	fedata = dev_get_drvdata(&dev->dev);
+
+	down(&fedata->socket_lock);
+	map = radix_tree_lookup(&fedata->socketpass_mappings, req->u.listen.id);
+	up(&fedata->socket_lock);
+	if (map == NULL)
+		goto out;
+
+	ret = inet_listen(map->sock, req->u.listen.backlog);
+
+out:
+	rsp = RING_GET_RESPONSE(&fedata->ring, fedata->ring.rsp_prod_pvt++);
+	rsp->req_id = req->req_id;
+	rsp->cmd = req->cmd;
+	rsp->u.listen.id = req->u.listen.id;
+	rsp->ret = ret;
 	return 0;
 }
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680591 — [PATCH v6 04/18] xen/pvcalls: xenbus state handling

Introduce the code to handle xenbus state changes.

Implement the probe function for the pvcalls backend. Write the
supported versions, max-page-order and function-calls nodes to xenstore,
as required by the protocol.

Introduce stub functions for disconnecting/connecting to a frontend.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 152 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 9044cf2..7bce750 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -25,20 +25,172 @@
 #include <xen/xenbus.h>
 #include <xen/interface/io/pvcalls.h>
 
+#define PVCALLS_VERSIONS "1"
+#define MAX_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER
+
 struct pvcalls_back_global {
 	struct list_head frontends;
 	struct semaphore frontends_lock;
 } pvcalls_back_global;
 
+static int backend_connect(struct xenbus_device *dev)
+{
+	return 0;
+}
+
+static int backend_disconnect(struct xenbus_device *dev)
+{
+	return 0;
+}
+
 static int pvcalls_back_probe(struct xenbus_device *dev,
 			      const struct xenbus_device_id *id)
 {
+	int err, abort;
+	struct xenbus_transaction xbt;
+
+again:
+	abort = 1;
+
+	err = xenbus_transaction_start(&xbt);
+	if (err) {
+		pr_warn("%s cannot create xenstore transaction\n", __func__);
+		return err;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "versions", "%s",
+			    PVCALLS_VERSIONS);
+	if (err) {
+		pr_warn("%s write out 'version' failed\n", __func__);
+		goto abort;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "max-page-order", "%u",
+			    MAX_RING_ORDER);
+	if (err) {
+		pr_warn("%s write out 'max-page-order' failed\n", __func__);
+		goto abort;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "function-calls",
+			    XENBUS_FUNCTIONS_CALLS);
+	if (err) {
+		pr_warn("%s write out 'function-calls' failed\n", __func__);
+		goto abort;
+	}
+
+	abort = 0;
+abort:
+	err = xenbus_transaction_end(xbt, abort);
+	if (err) {
+		if (err == -EAGAIN && !abort)
+			goto again;
+		pr_warn("%s cannot complete xenstore transaction\n", __func__);
+		return err;
+	}
+
+	xenbus_switch_state(dev, XenbusStateInitWait);
+
 	return 0;
 }
 
+static void set_backend_state(struct xenbus_device *dev,
+			      enum xenbus_state state)
+{
+	while (dev->state != state) {
+		switch (dev->state) {
+		case XenbusStateClosed:
+			switch (state) {
+			case XenbusStateInitWait:
+			case XenbusStateConnected:
+				xenbus_switch_state(dev, XenbusStateInitWait);
+				break;
+			case XenbusStateClosing:
+				xenbus_switch_state(dev, XenbusStateClosing);
+				break;
+			default:
+				__WARN();
+			}
+			break;
+		case XenbusStateInitWait:
+		case XenbusStateInitialised:
+			switch (state) {
+			case XenbusStateConnected:
+				backend_connect(dev);
+				xenbus_switch_state(dev, XenbusStateConnected);
+				break;
+			case XenbusStateClosing:
+			case XenbusStateClosed:
+				xenbus_switch_state(dev, XenbusStateClosing);
+				break;
+			default:
+				__WARN();
+			}
+			break;
+		case XenbusStateConnected:
+			switch (state) {
+			case XenbusStateInitWait:
+			case XenbusStateClosing:
+			case XenbusStateClosed:
+				down(&pvcalls_back_global.frontends_lock);
+				backend_disconnect(dev);
+				up(&pvcalls_back_global.frontends_lock);
+				xenbus_switch_state(dev, XenbusStateClosing);
+				break;
+			default:
+				__WARN();
+			}
+			break;
+		case XenbusStateClosing:
+			switch (state) {
+			case XenbusStateInitWait:
+			case XenbusStateConnected:
+			case XenbusStateClosed:
+				xenbus_switch_state(dev, XenbusStateClosed);
+				break;
+			default:
+				__WARN();
+			}
+			break;
+		default:
+			__WARN();
+		}
+	}
+}
+
 static void pvcalls_back_changed(struct xenbus_device *dev,
 				 enum xenbus_state frontend_state)
 {
+	switch (frontend_state) {
+	case XenbusStateInitialising:
+		set_backend_state(dev, XenbusStateInitWait);
+		break;
+
+	case XenbusStateInitialised:
+	case XenbusStateConnected:
+		set_backend_state(dev, XenbusStateConnected);
+		break;
+
+	case XenbusStateClosing:
+		set_backend_state(dev, XenbusStateClosing);
+		break;
+
+	case XenbusStateClosed:
+		set_backend_state(dev, XenbusStateClosed);
+		if (xenbus_dev_is_online(dev))
+			break;
+		device_unregister(&dev->dev);
+		break;
+	case XenbusStateUnknown:
+		set_backend_state(dev, XenbusStateClosed);
+		device_unregister(&dev->dev);
+		break;
+
+	default:
+		xenbus_dev_fatal(dev, -EINVAL, "saw state %d at frontend",
+				 frontend_state);
+		break;
+	}
 }
 
 static int pvcalls_back_remove(struct xenbus_device *dev)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680743 — Re: [PATCH v6 04/18] xen/pvcalls: xenbus state handling

On 03/07/17 23:08, Stefano Stabellini wrote:
> Introduce the code to handle xenbus state changes.
> 
> Implement the probe function for the pvcalls backend. Write the
> supported versions, max-page-order and function-calls nodes to xenstore,
> as required by the protocol.
> 
> Introduce stub functions for disconnecting/connecting to a frontend.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: boris.ostrovsky@oracle.com
> CC: jgross@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 152 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 152 insertions(+)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 9044cf2..7bce750 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -25,20 +25,172 @@
>  #include <xen/xenbus.h>
>  #include <xen/interface/io/pvcalls.h>
>  
> +#define PVCALLS_VERSIONS "1"
> +#define MAX_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER
> +
>  struct pvcalls_back_global {
>  	struct list_head frontends;
>  	struct semaphore frontends_lock;
>  } pvcalls_back_global;
>  
> +static int backend_connect(struct xenbus_device *dev)
> +{
> +	return 0;
> +}
> +
> +static int backend_disconnect(struct xenbus_device *dev)
> +{
> +	return 0;
> +}
> +
>  static int pvcalls_back_probe(struct xenbus_device *dev,
>  			      const struct xenbus_device_id *id)
>  {
> +	int err, abort;
> +	struct xenbus_transaction xbt;
> +
> +again:
> +	abort = 1;
> +
> +	err = xenbus_transaction_start(&xbt);
> +	if (err) {
> +		pr_warn("%s cannot create xenstore transaction\n", __func__);
> +		return err;
> +	}
> +
> +	err = xenbus_printf(xbt, dev->nodename, "versions", "%s",
> +			    PVCALLS_VERSIONS);
> +	if (err) {
> +		pr_warn("%s write out 'version' failed\n", __func__);

s/version/versions/ ?

> +		goto abort;
> +	}
> +
> +	err = xenbus_printf(xbt, dev->nodename, "max-page-order", "%u",
> +			    MAX_RING_ORDER);
> +	if (err) {
> +		pr_warn("%s write out 'max-page-order' failed\n", __func__);
> +		goto abort;
> +	}
> +
> +	err = xenbus_printf(xbt, dev->nodename, "function-calls",
> +			    XENBUS_FUNCTIONS_CALLS);
> +	if (err) {
> +		pr_warn("%s write out 'function-calls' failed\n", __func__);
> +		goto abort;
> +	}
> +
> +	abort = 0;
> +abort:
> +	err = xenbus_transaction_end(xbt, abort);
> +	if (err) {
> +		if (err == -EAGAIN && !abort)

Hmm, while I don't think xenbus_transaction_end() will ever
return -EAGAIN in the abort case I'm not sure you should limit
the retry loop to the non-abort case.

> +			goto again;
> +		pr_warn("%s cannot complete xenstore transaction\n", __func__);
> +		return err;
> +	}
> +
> +	xenbus_switch_state(dev, XenbusStateInitWait);

I don't think you should switch state in case of abort set, no?


Juergen

[toc] | [prev] | [next] | [standalone]

#1681782 — Re: [PATCH v6 04/18] xen/pvcalls: xenbus state handling

Many thanks for all the reviews!

On Tue, 4 Jul 2017, Juergen Gross wrote:
> On 03/07/17 23:08, Stefano Stabellini wrote:
> > Introduce the code to handle xenbus state changes.
> > 
> > Implement the probe function for the pvcalls backend. Write the
> > supported versions, max-page-order and function-calls nodes to xenstore,
> > as required by the protocol.
> > 
> > Introduce stub functions for disconnecting/connecting to a frontend.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > CC: boris.ostrovsky@oracle.com
> > CC: jgross@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 152 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 152 insertions(+)
> > 
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 9044cf2..7bce750 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -25,20 +25,172 @@
> >  #include <xen/xenbus.h>
> >  #include <xen/interface/io/pvcalls.h>
> >  
> > +#define PVCALLS_VERSIONS "1"
> > +#define MAX_RING_ORDER XENBUS_MAX_RING_GRANT_ORDER
> > +
> >  struct pvcalls_back_global {
> >  	struct list_head frontends;
> >  	struct semaphore frontends_lock;
> >  } pvcalls_back_global;
> >  
> > +static int backend_connect(struct xenbus_device *dev)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int backend_disconnect(struct xenbus_device *dev)
> > +{
> > +	return 0;
> > +}
> > +
> >  static int pvcalls_back_probe(struct xenbus_device *dev,
> >  			      const struct xenbus_device_id *id)
> >  {
> > +	int err, abort;
> > +	struct xenbus_transaction xbt;
> > +
> > +again:
> > +	abort = 1;
> > +
> > +	err = xenbus_transaction_start(&xbt);
> > +	if (err) {
> > +		pr_warn("%s cannot create xenstore transaction\n", __func__);
> > +		return err;
> > +	}
> > +
> > +	err = xenbus_printf(xbt, dev->nodename, "versions", "%s",
> > +			    PVCALLS_VERSIONS);
> > +	if (err) {
> > +		pr_warn("%s write out 'version' failed\n", __func__);
> 
> s/version/versions/ ?

OK


> > +		goto abort;
> > +	}
> > +
> > +	err = xenbus_printf(xbt, dev->nodename, "max-page-order", "%u",
> > +			    MAX_RING_ORDER);
> > +	if (err) {
> > +		pr_warn("%s write out 'max-page-order' failed\n", __func__);
> > +		goto abort;
> > +	}
> > +
> > +	err = xenbus_printf(xbt, dev->nodename, "function-calls",
> > +			    XENBUS_FUNCTIONS_CALLS);
> > +	if (err) {
> > +		pr_warn("%s write out 'function-calls' failed\n", __func__);
> > +		goto abort;
> > +	}
> > +
> > +	abort = 0;
> > +abort:
> > +	err = xenbus_transaction_end(xbt, abort);
> > +	if (err) {
> > +		if (err == -EAGAIN && !abort)
> 
> Hmm, while I don't think xenbus_transaction_end() will ever
> return -EAGAIN in the abort case I'm not sure you should limit
> the retry loop to the non-abort case.

Realistically, if we want to abort and get -EAGAIN, the best thing to do
is to get out (current behavior). The other option would be to keep
issuing xenbus_transaction_end(xbr, 1) in a loop until it succeeds, but
it seems more fragile to me.


> > +			goto again;
> > +		pr_warn("%s cannot complete xenstore transaction\n", __func__);
> > +		return err;
> > +	}
> > +
> > +	xenbus_switch_state(dev, XenbusStateInitWait);
> 
> I don't think you should switch state in case of abort set, no?

Good point, I'll change that.

[toc] | [prev] | [next] | [standalone]

#1680592 — [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions

We have one ioworker per socket. Each ioworker goes through the list of
outstanding read/write requests. Once all requests have been dealt with,
it returns.

We use one atomic counter per socket for "read" operations and one
for "write" operations to keep track of the reads/writes to do.

We also use one atomic counter ("io") per ioworker to keep track of how
many outstanding requests we have in total assigned to the ioworker. The
ioworker finishes when there are none.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
---
 drivers/xen/pvcalls-back.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 71a42fc..d59c2e4 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -96,8 +96,35 @@ static int pvcalls_back_release_active(struct xenbus_device *dev,
 				       struct pvcalls_fedata *fedata,
 				       struct sock_mapping *map);
 
+static void pvcalls_conn_back_read(void *opaque)
+{
+}
+
+static int pvcalls_conn_back_write(struct sock_mapping *map)
+{
+	return 0;
+}
+
 static void pvcalls_back_ioworker(struct work_struct *work)
 {
+	struct pvcalls_ioworker *ioworker = container_of(work,
+		struct pvcalls_ioworker, register_work);
+	struct sock_mapping *map = container_of(ioworker, struct sock_mapping,
+		ioworker);
+
+	while (atomic_read(&map->io) > 0) {
+		if (atomic_read(&map->release) > 0) {
+			atomic_set(&map->release, 0);
+			return;
+		}
+
+		if (atomic_read(&map->read) > 0)
+			pvcalls_conn_back_read(map);
+		if (atomic_read(&map->write) > 0)
+			pvcalls_conn_back_write(map);
+
+		atomic_dec(&map->io);
+	}
 }
 
 static int pvcalls_back_socket(struct xenbus_device *dev,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1680729 — Re: [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions

On 03/07/17 23:08, Stefano Stabellini wrote:
> We have one ioworker per socket. Each ioworker goes through the list of
> outstanding read/write requests. Once all requests have been dealt with,
> it returns.
> 
> We use one atomic counter per socket for "read" operations and one
> for "write" operations to keep track of the reads/writes to do.
> 
> We also use one atomic counter ("io") per ioworker to keep track of how
> many outstanding requests we have in total assigned to the ioworker. The
> ioworker finishes when there are none.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: boris.ostrovsky@oracle.com
> CC: jgross@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 71a42fc..d59c2e4 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -96,8 +96,35 @@ static int pvcalls_back_release_active(struct xenbus_device *dev,
>  				       struct pvcalls_fedata *fedata,
>  				       struct sock_mapping *map);
>  
> +static void pvcalls_conn_back_read(void *opaque)
> +{
> +}
> +
> +static int pvcalls_conn_back_write(struct sock_mapping *map)
> +{
> +	return 0;
> +}

Any reason for letting this function return int? I haven't spotted any
use of the return value in this or any later patch.


Juergen

[toc] | [prev] | [next] | [standalone]

#1681832 — Re: [PATCH v6 15/18] xen/pvcalls: implement the ioworker functions

On Tue, 4 Jul 2017, Juergen Gross wrote:
> On 03/07/17 23:08, Stefano Stabellini wrote:
> > We have one ioworker per socket. Each ioworker goes through the list of
> > outstanding read/write requests. Once all requests have been dealt with,
> > it returns.
> > 
> > We use one atomic counter per socket for "read" operations and one
> > for "write" operations to keep track of the reads/writes to do.
> > 
> > We also use one atomic counter ("io") per ioworker to keep track of how
> > many outstanding requests we have in total assigned to the ioworker. The
> > ioworker finishes when there are none.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > CC: boris.ostrovsky@oracle.com
> > CC: jgross@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 71a42fc..d59c2e4 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -96,8 +96,35 @@ static int pvcalls_back_release_active(struct xenbus_device *dev,
> >  				       struct pvcalls_fedata *fedata,
> >  				       struct sock_mapping *map);
> >  
> > +static void pvcalls_conn_back_read(void *opaque)
> > +{
> > +}
> > +
> > +static int pvcalls_conn_back_write(struct sock_mapping *map)
> > +{
> > +	return 0;
> > +}
> 
> Any reason for letting this function return int? I haven't spotted any
> use of the return value in this or any later patch.

No reason. I'll change it to void.

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web