Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752066AbdFUVeB (ORCPT ); Wed, 21 Jun 2017 17:34:01 -0400 Received: from mail.kernel.org ([198.145.29.99]:37440 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751789AbdFUVeA (ORCPT ); Wed, 21 Jun 2017 17:34:00 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 99DB2217C6 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: Wed, 21 Jun 2017 14:33:55 -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 v4 13/18] xen/pvcalls: implement release command In-Reply-To: Message-ID: References: <1497553787-3709-1-git-send-email-sstabellini@kernel.org> <1497553787-3709-13-git-send-email-sstabellini@kernel.org> 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: 1296 Lines: 32 On Tue, 20 Jun 2017, Boris Ostrovsky wrote: > > + > > +static int pvcalls_back_release_passive(struct xenbus_device *dev, > > + struct pvcalls_fedata *fedata, > > + 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(&fedata->socket_lock); > > + radix_tree_delete(&fedata->socketpass_mappings, mappass->id); > > + sock_release(mappass->sock); > > + flush_workqueue(mappass->wq); > > + destroy_workqueue(mappass->wq); > > + kfree(mappass); > > + up(&fedata->socket_lock); > > Can you raise the semaphore earlier, once the mapping is deleted from > the tree? Yes, I think it can. > Also, why are you not locking the tree in pvcalls_back_accept()? Good point! Although socket_lock is used to protect insertions and deletions to socketpass_mappings and socket_mappings, many of the sites that only access socket_mappings and socketpass_mappings without making modifications are left unprotected at the moment. I'll fix that, and to do it I'll move radix_tree_delete out of pvcalls_back_release_passive.