On Fri, May 3, 2013 at 6:09 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Fri, May 03, 2013 at 10:45:22AM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>>
>> 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 <pingf...@linux.vnet.ibm.com>
>> ---
>> 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
