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

[Valentin Priescu <vali.priescu@xxxxxxxxx>]

On Mon, May 12, 2014 at 11:39 AM, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> On 10/05/14 00:34, Valentin Priescu wrote:
>> From: Valentin Priescu <priescuv@xxxxxxxxxx>
>> @@ -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;
>

Will do.

>> +             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.
>

Yeah, I should do something like:
err = xen_blkif_disconnect(be->blkif);
if (err) {
    xenbus_dev_fatal(dev, err, "disconnecting %s",
            dev->otherend);
    break;
}

--
Valentin

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