On Fri, Aug 18, 2017 at 06:24:06PM +0100, Roger Pau Monné wrote: > On Fri, Aug 18, 2017 at 10:29:15AM -0400, annie li wrote: > > > > On 8/18/2017 5:14 AM, Roger Pau Monné wrote: > > > On Thu, Aug 17, 2017 at 06:43:46PM -0400, Annie Li wrote: > > > > If there is inflight I/O in any non-last queue, blkback returns -EBUSY > > > > directly, and never stops thread of remaining queue and processs them. > > > > When > > > > removing vbd device with lots of disk I/O load, some queues with > > > > inflight > > > > I/O still have blkback thread running even though the corresponding vbd > > > > device or guest is gone. > > > > And this could cause some problems, for example, if the backend device > > > > type > > > > is file, some loop devices and blkback thread always lingers there > > > > forever > > > > after guest is destroyed, and this causes failure of umounting > > > > repositories > > > > unless rebooting the dom0. So stop all threads properly and return > > > > -EBUSY > > > > if any queue has inflight I/O. > > > > > > > > Signed-off-by: Annie Li <annie...@oracle.com> > > > > Reviewed-by: Herbert van den Bergh <herbert.van.den.be...@oracle.com> > > > > Reviewed-by: Bhavesh Davda <bhavesh.da...@oracle.com> > > > > Reviewed-by: Adnan Misherfi <adnan.mishe...@oracle.com> > > > > --- > > > > drivers/block/xen-blkback/xenbus.c | 10 ++++++++-- > > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/block/xen-blkback/xenbus.c > > > > b/drivers/block/xen-blkback/xenbus.c > > > > index 792da68..2adb859 100644 > > > > --- a/drivers/block/xen-blkback/xenbus.c > > > > +++ b/drivers/block/xen-blkback/xenbus.c > > > > @@ -244,6 +244,7 @@ static int xen_blkif_disconnect(struct xen_blkif > > > > *blkif) > > > > { > > > > struct pending_req *req, *n; > > > > unsigned int j, r; > > > > + bool busy = false; > > > > for (r = 0; r < blkif->nr_rings; r++) { > > > > struct xen_blkif_ring *ring = &blkif->rings[r]; > > > > @@ -261,8 +262,10 @@ static int xen_blkif_disconnect(struct xen_blkif > > > > *blkif) > > > > * don't have any discard_io or other_io requests. So, > > > > checking > > > > * for inflight IO is enough. > > > > */ > > > > - if (atomic_read(&ring->inflight) > 0) > > > > - return -EBUSY; > > > > + if (atomic_read(&ring->inflight) > 0) { > > > > + busy = true; > > > > + continue; > > > > + } > > > I guess I'm missing something, but I don't see how this is solving the > > > problem described in the description. > > > > > > If the problem is that xen_blkif_disconnect returns without cleaning > > > all the queues, this patch keeps the current behavior, just that it > > > will try to remove more queues before returning, as opposed to > > > returning when finding the first busy queue. > > Before checking inflight, following code stops the blkback thread, > > if (ring->xenblkd) { > > kthread_stop(ring->xenblkd); > > wake_up(&ring->shutdown_wq); > > } > > This patch allows thread of every queue has the chance to get stopped. > > Otherwise, only thread of queue before(including) first busy one get > > stopped, threads of remaining queue will still run, and these blkthread and > > corresponding loop device will linger forever even after guest is destroyed. > > Thanks for the explanation: > > Acked-by: Roger Pau Monné <roger....@citrix.com>
Forgot to add, this needs to be backported to stable branches, so: Cc: sta...@vger.kernel.org Roger.