Re: [Qemu-devel] [PATCH v2 6/6] virtio-blk: release reference to RAM's memoryRegion

[liu ping fan]

On Fri, May 3, 2013 at 6:09 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, May 03, 2013 at 10:45:22AM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <address@hidden>
>>
>> virtio-blk will unref RAM's memoryRegion when the io-req has been
>> done. So we can avoid to call bdrv_drain_all() when RAM hot unplug.
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ---
>>  hw/block/dataplane/virtio-blk.c |   51 
>> +++++++++++++++++++++++++++++---------
>>  1 files changed, 39 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/block/dataplane/virtio-blk.c 
>> b/hw/block/dataplane/virtio-blk.c
>> index 3bb57d1..047e1df 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -35,6 +35,8 @@ enum {
>>
>>  typedef struct {
>>      struct iocb iocb;               /* Linux AIO control block */
>> +    MemoryRegion *mrs[VRING_MAX];
>> +    int mrs_cnt;
>>      QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
>>      unsigned int head;              /* vring descriptor index */
>>      struct iovec *bounce_iov;       /* used if guest buffers are unaligned 
>> */
>> @@ -121,6 +123,10 @@ static void complete_request(struct iocb *iocb, ssize_t 
>> ret, void *opaque)
>>       * transferred plus the status bytes.
>>       */
>>      vring_push(&s->vring, req->head, len + sizeof(hdr));
>> +    while (--req->mrs_cnt >= 0) {
>> +        memory_region_unref(req->mrs[req->mrs_cnt]);
>> +    }
>
> The vring has the property that every virtqueue element popped will be
> pushed back.
>
> Therefore it might be nicer to hide the MemoryRegion unref inside
> vring_push().  The user wouldn't have to know about MemoryRegion -
> that's especially nice since there are other devices besides virtio-blk
> (like virtio-net) that would need to be modified if we use the current
> approach.
>
How about use "head" as the key to pair vring_push/vring_pop, and keep
tracing the refer to MemoryRegion in Vring (resort to linked-list to
dynamically maintain mrs[] used by vring_push).  So the reference of
mr will be kept in Vring, totally transparent to caller.

Regards,
Pingfan

> Stefan