Patchwork [v3,1/2] block: add the support to drain throttled requests

login
register
mail settings
Submitter Zhiyong Wu
Date March 12, 2012, 2:53 p.m.
Message ID <1331564005-31070-1-git-send-email-zwu.kernel@gmail.com>
Download mbox | patch
Permalink /patch/146123/
State New
Headers show

Comments

Zhiyong Wu - March 12, 2012, 2:53 p.m.
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 block.c     |   21 ++++++++++++++++++++-
 block_int.h |    1 +
 2 files changed, 21 insertions(+), 1 deletions(-)
Zhiyong Wu - March 12, 2012, 3:25 p.m.
On Mon, Mar 12, 2012 at 11:05 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 12/03/2012 15:53, zwu.kernel@gmail.com ha scritto:
>> -    qemu_aio_flush();
>> +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
>> +        do {
>> +            qemu_co_queue_restart_all(&bs->throttled_reqs);
>> +            qemu_aio_flush();
>> +        } while (!qemu_co_queue_empty(&bs->throttled_reqs));
>> +    }
>
> Even this is not enough.  Block device 2 could start a throttled request
I think that this should be allowed here. It does not affect the final
result. All throttled requests for all disks are drained.
> on block device 1.  I'm sending a related series, I'll include yours in
> mine.
>
> Paolo
Paolo Bonzini - March 12, 2012, 3:30 p.m.
Il 12/03/2012 16:25, Zhi Yong Wu ha scritto:
> > > -    qemu_aio_flush();
> > > +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
> > > +        do {
> > > +            qemu_co_queue_restart_all(&bs->throttled_reqs);
> > > +            qemu_aio_flush();
> > > +        } while (!qemu_co_queue_empty(&bs->throttled_reqs));
> > > +    }
> >
> > Even this is not enough.  Block device 2 could start a throttled request
> > on block device 1.
>
> I think that this should be allowed here. It does not affect the final
> result. All throttled requests for all disks are drained.

Unfortunately, that depends on the order of block devices.  It needs to
be something like this:

    do {
        qemu_aio_flush();
        busy = false;
        QTAILQ_FOREACH(bs, &bdrv_states, list) {
            if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
                qemu_co_queue_restart_all(&bs->throttled_reqs);
                busy = true;
            }
        }
    } while (busy);


This means that bdrv_drain() cannot be implemented.  The device models
can "drain themselves", but for example monitor commands must still rely
on bdrv_drain_all().

Paolo

Patch

diff --git a/block.c b/block.c
index 52ffe14..b3cbcc3 100644
--- a/block.c
+++ b/block.c
@@ -853,6 +853,20 @@  void bdrv_close_all(void)
     }
 }
 
+/**
+ * Complete all pending requests for a block device
+ */
+void bdrv_drain(BlockDriverState *bs)
+{
+    do {
+        qemu_co_queue_restart_all(&bs->throttled_reqs);
+        qemu_aio_flush();
+    } while (!qemu_co_queue_empty(&bs->throttled_reqs));
+
+    assert(QLIST_EMPTY(&bs->tracked_requests));
+    assert(qemu_co_queue_empty(&bs->throttled_reqs));
+}
+
 /*
  * Wait for pending requests to complete across all BlockDriverStates
  *
@@ -863,7 +877,12 @@  void bdrv_drain_all(void)
 {
     BlockDriverState *bs;
 
-    qemu_aio_flush();
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        do {
+            qemu_co_queue_restart_all(&bs->throttled_reqs);
+            qemu_aio_flush();
+        } while (!qemu_co_queue_empty(&bs->throttled_reqs));
+    }
 
     /* If requests are still pending there is a bug somewhere */
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
diff --git a/block_int.h b/block_int.h
index b460c36..c624399 100644
--- a/block_int.h
+++ b/block_int.h
@@ -318,6 +318,7 @@  void qemu_aio_release(void *p);
 
 void bdrv_set_io_limits(BlockDriverState *bs,
                         BlockIOLimit *io_limits);
+void bdrv_drain(BlockDriverState *bs);
 
 #ifdef _WIN32
 int is_windows_drive(const char *filename);