[PATCH v2] block/nvme: Add driver statistics for access alignment and hw errors

Philippe Mathieu-Daudé posted 1 patch 3 years, 7 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201001162939.1567915-1-philmd@redhat.com
qapi/block-core.json | 24 +++++++++++++++++++++++-
block/nvme.c         | 27 +++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
[PATCH v2] block/nvme: Add driver statistics for access alignment and hw errors
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
Keep statistics of some hardware errors, and number of
aligned/unaligned I/O accesses.

QMP example booting a full RHEL 8.3 aarch64 guest:

{ "execute": "query-blockstats" }
{
    "return": [
        {
            "device": "",
            "node-name": "drive0",
            "stats": {
                "flush_total_time_ns": 6026948,
                "wr_highest_offset": 3383991230464,
                "wr_total_time_ns": 807450995,
                "failed_wr_operations": 0,
                "failed_rd_operations": 0,
                "wr_merged": 3,
                "wr_bytes": 50133504,
                "failed_unmap_operations": 0,
                "failed_flush_operations": 0,
                "account_invalid": false,
                "rd_total_time_ns": 1846979900,
                "flush_operations": 130,
                "wr_operations": 659,
                "rd_merged": 1192,
                "rd_bytes": 218244096,
                "account_failed": false,
                "idle_time_ns": 2678641497,
                "rd_operations": 7406,
            },
            "driver-specific": {
                "driver": "nvme",
                "completion-errors": 0,
                "unaligned-accesses": 2959,
                "aligned-accesses": 4477
            },
            "qdev": "/machine/peripheral-anon/device[0]/virtio-backend"
        }
    ]
}

Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: 'access-nb' -> 'accesses' (Stefan)
---
 qapi/block-core.json | 24 +++++++++++++++++++++++-
 block/nvme.c         | 27 +++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 86ed72ef9f..dec17e3036 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -941,6 +941,27 @@
       'discard-nb-failed': 'uint64',
       'discard-bytes-ok': 'uint64' } }
 
+##
+# @BlockStatsSpecificNvme:
+#
+# NVMe driver statistics
+#
+# @completion-errors: The number of completion errors.
+#
+# @aligned-accesses: The number of aligned accesses performed by
+#                    the driver.
+#
+# @unaligned-accesses: The number of unaligned accesses performed by
+#                      the driver.
+#
+# Since: 5.2
+##
+{ 'struct': 'BlockStatsSpecificNvme',
+  'data': {
+      'completion-errors': 'uint64',
+      'aligned-accesses': 'uint64',
+      'unaligned-accesses': 'uint64' } }
+
 ##
 # @BlockStatsSpecific:
 #
@@ -953,7 +974,8 @@
   'discriminator': 'driver',
   'data': {
       'file': 'BlockStatsSpecificFile',
-      'host_device': 'BlockStatsSpecificFile' } }
+      'host_device': 'BlockStatsSpecificFile',
+      'nvme': 'BlockStatsSpecificNvme' } }
 
 ##
 # @BlockStats:
diff --git a/block/nvme.c b/block/nvme.c
index f4f27b6da7..ba6d066067 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -133,6 +133,12 @@ struct BDRVNVMeState {
 
     /* PCI address (required for nvme_refresh_filename()) */
     char *device;
+
+    struct {
+        uint64_t completion_errors;
+        uint64_t aligned_accesses;
+        uint64_t unaligned_accesses;
+    } stats;
 };
 
 #define NVME_BLOCK_OPT_DEVICE "device"
@@ -389,6 +395,9 @@ static bool nvme_process_completion(NVMeQueuePair *q)
             break;
         }
         ret = nvme_translate_error(c);
+        if (ret) {
+            s->stats.completion_errors++;
+        }
         q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
         if (!q->cq.head) {
             q->cq_phase = !q->cq_phase;
@@ -1146,8 +1155,10 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     assert(QEMU_IS_ALIGNED(bytes, s->page_size));
     assert(bytes <= s->max_transfer);
     if (nvme_qiov_aligned(bs, qiov)) {
+        s->stats.aligned_accesses++;
         return nvme_co_prw_aligned(bs, offset, bytes, qiov, is_write, flags);
     }
+    s->stats.unaligned_accesses++;
     trace_nvme_prw_buffered(s, offset, bytes, qiov->niov, is_write);
     buf = qemu_try_memalign(s->page_size, bytes);
 
@@ -1443,6 +1454,21 @@ static void nvme_unregister_buf(BlockDriverState *bs, void *host)
     qemu_vfio_dma_unmap(s->vfio, host);
 }
 
+static BlockStatsSpecific *nvme_get_specific_stats(BlockDriverState *bs)
+{
+    BlockStatsSpecific *stats = g_new(BlockStatsSpecific, 1);
+    BDRVNVMeState *s = bs->opaque;
+
+    stats->driver = BLOCKDEV_DRIVER_NVME;
+    stats->u.nvme = (BlockStatsSpecificNvme) {
+        .completion_errors = s->stats.completion_errors,
+        .aligned_accesses = s->stats.aligned_accesses,
+        .unaligned_accesses = s->stats.unaligned_accesses,
+    };
+
+    return stats;
+}
+
 static const char *const nvme_strong_runtime_opts[] = {
     NVME_BLOCK_OPT_DEVICE,
     NVME_BLOCK_OPT_NAMESPACE,
@@ -1476,6 +1502,7 @@ static BlockDriver bdrv_nvme = {
     .bdrv_refresh_filename    = nvme_refresh_filename,
     .bdrv_refresh_limits      = nvme_refresh_limits,
     .strong_runtime_opts      = nvme_strong_runtime_opts,
+    .bdrv_get_specific_stats  = nvme_get_specific_stats,
 
     .bdrv_detach_aio_context  = nvme_detach_aio_context,
     .bdrv_attach_aio_context  = nvme_attach_aio_context,
-- 
2.26.2

Re: [PATCH v2] block/nvme: Add driver statistics for access alignment and hw errors
Posted by Markus Armbruster 3 years, 7 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Keep statistics of some hardware errors, and number of
> aligned/unaligned I/O accesses.
>
> QMP example booting a full RHEL 8.3 aarch64 guest:
>
> { "execute": "query-blockstats" }
> {
>     "return": [
>         {
>             "device": "",
>             "node-name": "drive0",
>             "stats": {
>                 "flush_total_time_ns": 6026948,
>                 "wr_highest_offset": 3383991230464,
>                 "wr_total_time_ns": 807450995,
>                 "failed_wr_operations": 0,
>                 "failed_rd_operations": 0,
>                 "wr_merged": 3,
>                 "wr_bytes": 50133504,
>                 "failed_unmap_operations": 0,
>                 "failed_flush_operations": 0,
>                 "account_invalid": false,
>                 "rd_total_time_ns": 1846979900,
>                 "flush_operations": 130,
>                 "wr_operations": 659,
>                 "rd_merged": 1192,
>                 "rd_bytes": 218244096,
>                 "account_failed": false,
>                 "idle_time_ns": 2678641497,
>                 "rd_operations": 7406,
>             },
>             "driver-specific": {
>                 "driver": "nvme",
>                 "completion-errors": 0,
>                 "unaligned-accesses": 2959,
>                 "aligned-accesses": 4477
>             },
>             "qdev": "/machine/peripheral-anon/device[0]/virtio-backend"
>         }
>     ]
> }
>
> Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: 'access-nb' -> 'accesses' (Stefan)
> ---
>  qapi/block-core.json | 24 +++++++++++++++++++++++-
>  block/nvme.c         | 27 +++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 86ed72ef9f..dec17e3036 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -941,6 +941,27 @@
>        'discard-nb-failed': 'uint64',
>        'discard-bytes-ok': 'uint64' } }
>  
> +##
> +# @BlockStatsSpecificNvme:
> +#
> +# NVMe driver statistics
> +#
> +# @completion-errors: The number of completion errors.
> +#
> +# @aligned-accesses: The number of aligned accesses performed by
> +#                    the driver.
> +#
> +# @unaligned-accesses: The number of unaligned accesses performed by
> +#                      the driver.
> +#
> +# Since: 5.2
> +##
> +{ 'struct': 'BlockStatsSpecificNvme',
> +  'data': {
> +      'completion-errors': 'uint64',
> +      'aligned-accesses': 'uint64',
> +      'unaligned-accesses': 'uint64' } }
> +
>  ##
>  # @BlockStatsSpecific:
>  #
> @@ -953,7 +974,8 @@
>    'discriminator': 'driver',
>    'data': {
>        'file': 'BlockStatsSpecificFile',
> -      'host_device': 'BlockStatsSpecificFile' } }
> +      'host_device': 'BlockStatsSpecificFile',
> +      'nvme': 'BlockStatsSpecificNvme' } }
>  
>  ##
>  # @BlockStats:

Acked-by: Markus Armbruster <armbru@redhat.com>


Re: [PATCH v2] block/nvme: Add driver statistics for access alignment and hw errors
Posted by Stefan Hajnoczi 3 years, 7 months ago
On Thu, Oct 01, 2020 at 06:29:39PM +0200, Philippe Mathieu-Daudé wrote:
> Keep statistics of some hardware errors, and number of
> aligned/unaligned I/O accesses.
> 
> QMP example booting a full RHEL 8.3 aarch64 guest:
> 
> { "execute": "query-blockstats" }
> {
>     "return": [
>         {
>             "device": "",
>             "node-name": "drive0",
>             "stats": {
>                 "flush_total_time_ns": 6026948,
>                 "wr_highest_offset": 3383991230464,
>                 "wr_total_time_ns": 807450995,
>                 "failed_wr_operations": 0,
>                 "failed_rd_operations": 0,
>                 "wr_merged": 3,
>                 "wr_bytes": 50133504,
>                 "failed_unmap_operations": 0,
>                 "failed_flush_operations": 0,
>                 "account_invalid": false,
>                 "rd_total_time_ns": 1846979900,
>                 "flush_operations": 130,
>                 "wr_operations": 659,
>                 "rd_merged": 1192,
>                 "rd_bytes": 218244096,
>                 "account_failed": false,
>                 "idle_time_ns": 2678641497,
>                 "rd_operations": 7406,
>             },
>             "driver-specific": {
>                 "driver": "nvme",
>                 "completion-errors": 0,
>                 "unaligned-accesses": 2959,
>                 "aligned-accesses": 4477
>             },
>             "qdev": "/machine/peripheral-anon/device[0]/virtio-backend"
>         }
>     ]
> }
> 
> Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: 'access-nb' -> 'accesses' (Stefan)
> ---
>  qapi/block-core.json | 24 +++++++++++++++++++++++-
>  block/nvme.c         | 27 +++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan