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@xxxxxxxxxxxxxxx
> Reported-by: Glenn Enright <glenn@xxxxxxxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Tested-by: Steven Haigh <netwiz@xxxxxxxxx>
> ---
> 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