From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail1.bemta6.messagelabs.com ([193.109.254.110]:55802 "EHLO mail1.bemta6.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751264AbdEPJ2d (ORCPT ); Tue, 16 May 2017 05:28:33 -0400 From: Dietmar Hahn To: xen-devel@lists.xen.org Cc: Juergen Gross , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, roger.pau@citrix.com, netwiz@crc.id.au, stable@vger.kernel.org Subject: Re: [Xen-devel] [PATCH 1/3] xen/blkback: fix disconnect while I/Os in flight Date: Tue, 16 May 2017 11:28:14 +0200 Message-ID: <1838893.p5VEmPBDei@amur> In-Reply-To: <20170516062320.22008-2-jgross@suse.com> References: <20170516062320.22008-1-jgross@suse.com> <20170516062320.22008-2-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: stable-owner@vger.kernel.org List-ID: Am Dienstag, 16. Mai 2017, 08:23:18 schrieb Juergen Gross: > Today disconnecting xen-blkback is broken in case there are still > I/Os in flight: xen_blkif_disconnect() will bail out early without > releasing all resources in the hope it will be called again when > the last request has terminated. This, however, won't happen as > xen_blkif_free() won't be called on termination of the last running > request: xen_blkif_put() won't decrement the blkif refcnt to 0 as > xen_blkif_disconnect() didn't finish before thus some xen_blkif_put() > calls in xen_blkif_disconnect() didn't happen. > > To solve this deadlock xen_blkif_disconnect() and > xen_blkif_alloc_rings() shouldn't use xen_blkif_put() and > xen_blkif_get() but use some other way to do their accounting of > resources. > > This at once fixes another error in xen_blkif_disconnect(): when it > returned early with -EBUSY for another ring than 0 it would call > xen_blkif_put() again for already handled rings on a subsequent call. > This will lead to inconsistencies in the refcnt handling. > > Cc: stable@vger.kernel.org > Reported-by: Glenn Enright > Signed-off-by: Juergen Gross > Tested-by: Steven Haigh > --- > drivers/block/xen-blkback/common.h | 1 + > drivers/block/xen-blkback/xenbus.c | 7 +++++-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h > index dea61f6ab8cb..953f38802333 100644 > --- a/drivers/block/xen-blkback/common.h > +++ b/drivers/block/xen-blkback/common.h > @@ -281,6 +281,7 @@ struct xen_blkif_ring { > > wait_queue_head_t wq; > atomic_t inflight; > + int active; Maybe better using bool? Dietmar. > /* One thread per blkif ring. */ > struct task_struct *xenblkd; > unsigned int waiting_reqs; > diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c > index 1f3dfaa54d87..e68df9de8858 100644 > --- a/drivers/block/xen-blkback/xenbus.c > +++ b/drivers/block/xen-blkback/xenbus.c > @@ -159,7 +159,7 @@ static int xen_blkif_alloc_rings(struct xen_blkif *blkif) > init_waitqueue_head(&ring->shutdown_wq); > ring->blkif = blkif; > ring->st_print = jiffies; > - xen_blkif_get(blkif); > + ring->active = 1; > } > > return 0; > @@ -249,6 +249,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif) > struct xen_blkif_ring *ring = &blkif->rings[r]; > unsigned int i = 0; > > + if (!ring->active) > + continue; > + > if (ring->xenblkd) { > kthread_stop(ring->xenblkd); > wake_up(&ring->shutdown_wq); > @@ -296,7 +299,7 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif) > BUG_ON(ring->free_pages_num != 0); > BUG_ON(ring->persistent_gnt_c != 0); > WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages)); > - xen_blkif_put(blkif); > + ring->active = 0; > } > blkif->nr_ring_pages = 0; > /* > -- Company details: http://ts.fujitsu.com/imprint.html