Re: [Xen-devel] [PATCH] xen-blkback: defer freeing blkif to avoid blocking xenwatch

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 10/05/14 00:34, Valentin Priescu wrote:
> From: Valentin Priescu <priescuv@xxxxxxxxxx>
> 
> Currently xenwatch blocks in VBD disconnect, waiting for all pending I/O
> requests to finish. If the VBD is attached to a hot-swappable disk, then
> xenwatch can hang for a long period of time, stalling other watches.
>
[...]
> 
> With this patch, when there is still pending I/O, the actual disconnect
> is done by the last reference holder (last pending I/O request). In this
> case, xenwatch doesn't block indefinitely.

[...]

> @@ -239,19 +253,26 @@ static void xen_blkif_disconnect(struct xen_blkif 
> *blkif)
>               blkif->xenblkd = NULL;
>       }
>  
> -     atomic_dec(&blkif->refcnt);
> -     wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
> -     atomic_inc(&blkif->refcnt);
> +     /* The above kthread_stop() guarantees that at this point we
> +      * don't have any discard_io or other_io requests. So, checking
> +      * for inflight IO is enough.
> +      */
> +     if (!atomic_read(&blkif->inflight)) {

if (atomic_read(&blkif->inflight > 0)
    return -EBUSY;

> +             if (blkif->irq) {
> +                     unbind_from_irqhandler(blkif->irq, blkif);
> +                     blkif->irq = 0;
> +             }
>  
> -     if (blkif->irq) {
> -             unbind_from_irqhandler(blkif->irq, blkif);
> -             blkif->irq = 0;
> -     }
> +             if (blkif->blk_rings.common.sring) {
> +                     xenbus_unmap_ring_vfree(blkif->be->dev,
> +                             blkif->blk_ring);
> +                     blkif->blk_rings.common.sring = NULL;
> +             }
>  
> -     if (blkif->blk_rings.common.sring) {
> -             xenbus_unmap_ring_vfree(blkif->be->dev, blkif->blk_ring);
> -             blkif->blk_rings.common.sring = NULL;
> +             return 0;
>       }
> +
> +     return -EBUSY;
>  }
[...]
> @@ -700,7 +720,9 @@ static void frontend_changed(struct xenbus_device *dev,
>                * Enforce precondition before potential leak point.
>                * xen_blkif_disconnect() is idempotent.
>                */
> -             xen_blkif_disconnect(be->blkif);
> +             err = xen_blkif_disconnect(be->blkif);
> +             if (err)
> +                     break;
>  
>               err = connect_ring(be);
>               if (err)

If xen_blkif_disconnect() returns -EBUSY, nothing will trigger the
reconnect.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel