Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757362AbdIHXJF (ORCPT ); Fri, 8 Sep 2017 19:09:05 -0400 Received: from mail.kernel.org ([198.145.29.99]:41196 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757292AbdIHXJD (ORCPT ); Fri, 8 Sep 2017 19:09:03 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D1E2214C5 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: Fri, 8 Sep 2017 16:09:02 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Boris Ostrovsky cc: Stefano Stabellini , xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, jgross@suse.com, Stefano Stabellini Subject: Re: [PATCH v3 12/13] xen/pvcalls: implement release command In-Reply-To: <2fd1b3f3-28aa-1958-7522-766c7d92e8d3@oracle.com> Message-ID: References: <1501541855-7354-1-git-send-email-sstabellini@kernel.org> <1501541855-7354-12-git-send-email-sstabellini@kernel.org> <2fd1b3f3-28aa-1958-7522-766c7d92e8d3@oracle.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: 2592 Lines: 80 On Tue, 15 Aug 2017, Boris Ostrovsky wrote: > On 07/31/2017 06:57 PM, Stefano Stabellini wrote: > > Send PVCALLS_RELEASE to the backend and wait for a reply. Take both > > in_mutex and out_mutex to avoid concurrent accesses. Then, free the > > socket. > > > > For passive sockets, check whether we have already pre-allocated an > > active socket for the purpose of being accepted. If so, free that as > > well. > > > > Signed-off-by: Stefano Stabellini > > CC: boris.ostrovsky@oracle.com > > CC: jgross@suse.com > > --- > > drivers/xen/pvcalls-front.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ > > drivers/xen/pvcalls-front.h | 1 + > > 2 files changed, 89 insertions(+) > > > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > > index 1c975d6..775a6d2 100644 > > --- a/drivers/xen/pvcalls-front.c > > +++ b/drivers/xen/pvcalls-front.c > > @@ -192,6 +192,23 @@ static irqreturn_t pvcalls_front_conn_handler(int irq, void *sock_map) > > return IRQ_HANDLED; > > } > > > > +static void pvcalls_front_free_map(struct pvcalls_bedata *bedata, > > + struct sock_mapping *map) > > +{ > > + int i; > > + > > + spin_lock(&bedata->pvcallss_lock); > > + if (!list_empty(&map->list)) > > + list_del_init(&map->list); > > + spin_unlock(&bedata->pvcallss_lock); > > + > > + for (i = 0; i < (1 << map->active.ring->ring_order); i++) > > + gnttab_end_foreign_access(map->active.ring->ref[i], 0, 0); > > + gnttab_end_foreign_access(map->active.ref, 0, 0); > > + free_page((unsigned long)map->active.ring); > > + unbind_from_irqhandler(map->active.irq, map); > > Would it better to first unbind the handler? Any chance an interrupt > might come in? Fair enough, I'll do that. > > +} > > + > > int pvcalls_front_socket(struct socket *sock) > > { > > struct pvcalls_bedata *bedata; > > @@ -853,6 +870,77 @@ unsigned int pvcalls_front_poll(struct file *file, struct socket *sock, > > return pvcalls_front_poll_passive(file, bedata, map, wait); > > } > > > > +int pvcalls_front_release(struct socket *sock) > > +{ > > + struct pvcalls_bedata *bedata; > > + struct sock_mapping *map; > > + int req_id, notify, ret; > > + struct xen_pvcalls_request *req; > > + > > + if (!pvcalls_front_dev) > > + return -EIO; > > + bedata = dev_get_drvdata(&pvcalls_front_dev->dev); > > + > > + if (sock->sk == NULL) > > + return 0; > > This can go above bedata access. Yes, good idea. > (You are going to address locking here so I won't review the rest) Yes, I will. Thanks for the review! And sorry for taking so long to come back to you.