Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754187AbdFNArm (ORCPT ); Tue, 13 Jun 2017 20:47:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:32952 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753947AbdFNArk (ORCPT ); Tue, 13 Jun 2017 20:47:40 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A589239A8 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=sstabellini@kernel.org Date: Tue, 13 Jun 2017 17:47:38 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Juergen Gross cc: Stefano Stabellini , xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, boris.ostrovsky@oracle.com, Stefano Stabellini Subject: Re: [PATCH v3 13/18] xen/pvcalls: implement release command In-Reply-To: <18386730-7ef1-ed3e-0a0d-845bd8cbb8ef@suse.com> Message-ID: References: <1496431915-20774-1-git-send-email-sstabellini@kernel.org> <1496431915-20774-13-git-send-email-sstabellini@kernel.org> <18386730-7ef1-ed3e-0a0d-845bd8cbb8ef@suse.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3655 Lines: 112 On Tue, 13 Jun 2017, Juergen Gross wrote: > On 02/06/17 21:31, Stefano Stabellini wrote: > > Release both active and passive sockets. For active sockets, make sure > > to avoid possible conflicts with the ioworker reading/writing to those > > sockets concurrently. Set map->release to let the ioworker know > > atomically that the socket will be released soon, then wait until the > > ioworker finishes (flush_work). > > > > Unmap indexes pages and data rings. > > > > Signed-off-by: Stefano Stabellini > > CC: boris.ostrovsky@oracle.com > > CC: jgross@suse.com > > --- > > drivers/xen/pvcalls-back.c | 72 +++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 71 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c > > index 82f350d..b541887 100644 > > --- a/drivers/xen/pvcalls-back.c > > +++ b/drivers/xen/pvcalls-back.c > > @@ -265,13 +265,83 @@ static int pvcalls_back_release_active(struct xenbus_device *dev, > > struct pvcalls_fedata *priv, > > struct sock_mapping *map) > > { > > + disable_irq(map->irq); > > + if (map->sock->sk != NULL) { > > + write_lock_bh(&map->sock->sk->sk_callback_lock); > > + map->sock->sk->sk_user_data = NULL; > > + map->sock->sk->sk_data_ready = map->saved_data_ready; > > + write_unlock_bh(&map->sock->sk->sk_callback_lock); > > + } > > + > > + atomic_set(&map->release, 1); > > + flush_work(&map->ioworker.register_work); > > + > > + down(&priv->socket_lock); > > + list_del(&map->list); > > + up(&priv->socket_lock); > > + > > + xenbus_unmap_ring_vfree(dev, (void *)map->bytes); > > + xenbus_unmap_ring_vfree(dev, (void *)map->ring); > > I don't think you need the casts to (void*) here. OK > > + unbind_from_irqhandler(map->irq, map); > > + > > + sock_release(map->sock); > > + kfree(map); > > + > > + return 0; > > +} > > + > > +static int pvcalls_back_release_passive(struct xenbus_device *dev, > > + struct pvcalls_fedata *priv, > > + struct sockpass_mapping *mappass) > > +{ > > + if (mappass->sock->sk != NULL) { > > + write_lock_bh(&mappass->sock->sk->sk_callback_lock); > > + mappass->sock->sk->sk_user_data = NULL; > > + mappass->sock->sk->sk_data_ready = mappass->saved_data_ready; > > + write_unlock_bh(&mappass->sock->sk->sk_callback_lock); > > + } > > + down(&priv->socket_lock); > > + radix_tree_delete(&priv->socketpass_mappings, mappass->id); > > + sock_release(mappass->sock); > > + flush_workqueue(mappass->wq); > > + destroy_workqueue(mappass->wq); > > + kfree(mappass); > > + up(&priv->socket_lock); > > + > > return 0; > > } > > > > static int pvcalls_back_release(struct xenbus_device *dev, > > struct xen_pvcalls_request *req) > > { > > - return 0; > > + struct pvcalls_fedata *priv; > > + struct sock_mapping *map, *n; > > + struct sockpass_mapping *mappass; > > + int ret = 0; > > + struct xen_pvcalls_response *rsp; > > + > > + priv = dev_get_drvdata(&dev->dev); > > + > > + list_for_each_entry_safe(map, n, &priv->socket_mappings, list) { > > + if (map->id == req->u.release.id) { > > + ret = pvcalls_back_release_active(dev, priv, map); > > + goto out; > > + } > > + } > > + mappass = radix_tree_lookup(&priv->socketpass_mappings, > > + req->u.release.id); > > + if (mappass != NULL) { > > + ret = pvcalls_back_release_passive(dev, priv, mappass); > > + goto out; > > + } > > + > > +out: > > + rsp = RING_GET_RESPONSE(&priv->ring, priv->ring.rsp_prod_pvt++); > > + rsp->req_id = req->req_id; > > + rsp->u.release.id = req->u.release.id; > > + rsp->cmd = req->cmd; > > + rsp->ret = ret; > > + return ret; > > return 0; OK