qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [RFC PATCH] block/qapi: Optimize the query function of the


From: Dou Liyang
Subject: [Qemu-devel] [RFC PATCH] block/qapi: Optimize the query function of the blockstats
Date: Thu, 8 Dec 2016 15:56:42 +0800

In the query function named qmp_query_blockstats, for each block/node,
the process is as shown below:
the 0 func determines which the lookup queue will be used by the 1 func,
then gets the next object by the 1 func and calls the 2. The 2 func calls
the 3 and 4 func. There are some judgement and work that is useless and
repetitive.

+--------------+      +---------------------+
 | 1            |      | 4.                  |
 |next_query_bds|      |bdrv_query_bds_stats +---+
 |              |      |                     |   |
 +--------^-----+      +-------------^-------+   |
          |                          |           |
+---------+----------+      +--------+-------+   |
| 0.                 |      | 2.             |   |
|qmp_query_blockstats+------>bdrv_query_stats<----
|                    |      |                |
+--------------------+      +--------+-------+
                                     |
                       +-------------v-------+
                       | 3.                  |
                       |bdrv_query_blk_stats |
                       |                     |
                       +---------------------+

So, we think its logic is complex, and its readability is low.
There is no need to do so. Just let the function do its own thing.

Now, we optimize them make sure that:
1. the func named bdrv_query_bds_stats do the work for the BDS.
2. the func named bdrv_query_blk_stats do the work for the Blocks.
3. the query func just call what the want actually in need.
                                    +--------------+
                                    |              |
                           +--------v-----------+  |
                       +--->  3.                |  |
+-------------------+  |   |bdrv_query_bds_stats+--+
| 1.                +--+   |                    |
|                   +      +--------------------+
|qmp_query_blockstats--+
|                   |  |
+-------------------+  |   +--------------------+
                       |   | 2.                 |
                       +--->                    |
                           |bdrv_query_blk_stats|
                           |                    |
                           +--------------------+

Signed-off-by: Dou Liyang <address@hidden>
---
 block/qapi.c | 106 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index a62e862..090452b 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -357,14 +357,14 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo 
**p_info,
     qapi_free_BlockInfo(info);
 }
 
-static BlockStats *bdrv_query_stats(BlockBackend *blk,
-                                    const BlockDriverState *bs,
-                                    bool query_backing);
-
-static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
+static BlockStats *bdrv_query_blk_stats(BlockStats *s, BlockBackend *blk)
 {
     BlockAcctStats *stats = blk_get_stats(blk);
     BlockAcctTimedStats *ts = NULL;
+    BlockDeviceStats *ds = s->stats;
+
+    s->has_device = true;
+    s->device = g_strdup(blk_name(blk));
 
     ds->rd_bytes = stats->nr_bytes[BLOCK_ACCT_READ];
     ds->wr_bytes = stats->nr_bytes[BLOCK_ACCT_WRITE];
@@ -426,11 +426,24 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 
BlockBackend *blk)
         dev_stats->avg_wr_queue_depth =
             block_acct_queue_depth(ts, BLOCK_ACCT_WRITE);
     }
+
+    return s;
 }
 
-static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs,
+static BlockStats *bdrv_query_bds_stats(BlockStats *s,
+                                 const BlockDriverState *bs,
                                  bool query_backing)
 {
+
+    if (!s) {
+        s = g_malloc0(sizeof(*s));
+        s->stats = g_malloc0(sizeof(*s->stats));
+    }
+
+    if (!bs) {
+        return s;
+    }
+
     if (bdrv_get_node_name(bs)[0]) {
         s->has_node_name = true;
         s->node_name = g_strdup(bdrv_get_node_name(bs));
@@ -440,32 +453,12 @@ static void bdrv_query_bds_stats(BlockStats *s, const 
BlockDriverState *bs,
 
     if (bs->file) {
         s->has_parent = true;
-        s->parent = bdrv_query_stats(NULL, bs->file->bs, query_backing);
+        s->parent = bdrv_query_bds_stats(NULL, bs->file->bs, query_backing);
     }
 
     if (query_backing && bs->backing) {
         s->has_backing = true;
-        s->backing = bdrv_query_stats(NULL, bs->backing->bs, query_backing);
-    }
-
-}
-
-static BlockStats *bdrv_query_stats(BlockBackend *blk,
-                                    const BlockDriverState *bs,
-                                    bool query_backing)
-{
-    BlockStats *s;
-
-    s = g_malloc0(sizeof(*s));
-    s->stats = g_malloc0(sizeof(*s->stats));
-
-    if (blk) {
-        s->has_device = true;
-        s->device = g_strdup(blk_name(blk));
-        bdrv_query_blk_stats(s->stats, blk);
-    }
-    if (bs) {
-        bdrv_query_bds_stats(s, bs, query_backing);
+        s->backing = bdrv_query_bds_stats(NULL, bs->backing->bs, 
query_backing);
     }
 
     return s;
@@ -494,42 +487,49 @@ BlockInfoList *qmp_query_block(Error **errp)
     return head;
 }
 
-static bool next_query_bds(BlockBackend **blk, BlockDriverState **bs,
-                           bool query_nodes)
-{
-    if (query_nodes) {
-        *bs = bdrv_next_node(*bs);
-        return !!*bs;
-    }
-
-    *blk = blk_next(*blk);
-    *bs = *blk ? blk_bs(*blk) : NULL;
-
-    return !!*blk;
-}
-
 BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
                                      bool query_nodes,
                                      Error **errp)
 {
     BlockStatsList *head = NULL, **p_next = &head;
-    BlockBackend *blk = NULL;
+    BlockBackend *blk;
     BlockDriverState *bs = NULL;
 
     /* Just to be safe if query_nodes is not always initialized */
-    query_nodes = has_query_nodes && query_nodes;
+    if (has_query_nodes && query_nodes) {
+        while ((bs = bdrv_next_node(bs))) {
+            BlockStatsList *info = g_malloc0(sizeof(*info));
+            AioContext *ctx = bdrv_get_aio_context(bs);
+            BlockStats *s;
 
-    while (next_query_bds(&blk, &bs, query_nodes)) {
-        BlockStatsList *info = g_malloc0(sizeof(*info));
-        AioContext *ctx = blk ? blk_get_aio_context(blk)
-                              : bdrv_get_aio_context(bs);
+            s = g_malloc0(sizeof(*s));
+            s->stats = g_malloc0(sizeof(*s->stats));
 
-        aio_context_acquire(ctx);
-        info->value = bdrv_query_stats(blk, bs, !query_nodes);
-        aio_context_release(ctx);
+            aio_context_acquire(ctx);
+            info->value = bdrv_query_bds_stats(s, bs, false);
+            aio_context_release(ctx);
 
-        *p_next = info;
-        p_next = &info->next;
+            *p_next = info;
+             p_next = &info->next;
+        }
+    } else {
+        for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+            BlockStatsList *info = g_malloc0(sizeof(*info));
+            AioContext *ctx = blk_get_aio_context(blk);
+            BlockStats *s;
+
+            s = g_malloc0(sizeof(*s));
+            s->stats = g_malloc0(sizeof(*s->stats));
+            bs = blk_bs(blk);
+
+            aio_context_acquire(ctx);
+            s = bdrv_query_blk_stats(s, blk);
+            info->value = bdrv_query_bds_stats(s, bs, true);
+            aio_context_release(ctx);
+
+            *p_next = info;
+            p_next = &info->next;
+        }
     }
 
     return head;
-- 
2.5.5






reply via email to

[Prev in Thread] Current Thread [Next in Thread]