You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2018/12/05 14:34:34 UTC

[mynewt-core] 03/08: hw/bus: Enable bus driver statistics

This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git

commit cffe5e9f37379b25c9811f734d7aad1417f404da
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Dec 3 16:09:35 2018 +0100

    hw/bus: Enable bus driver statistics
    
    This adds option to enable statistics for bus driver. There are two
    types of statistics which can be used:
    - per-device statistics (default)
    - per-node statistics (optional)
    
    For now cumulative number of reads, writes and errors for those are
    counted as well as number of lock timeouts.
    
    Stats for bus device are registered with bus device name prefixed by
    "bd_" while bus nodes are prefixed with "bn_".
---
 hw/bus/include/bus/bus_driver.h | 21 +++++++++++++
 hw/bus/src/bus.c                | 67 +++++++++++++++++++++++++++++++++++++++++
 hw/bus/syscfg.yml               | 12 ++++++++
 3 files changed, 100 insertions(+)

diff --git a/hw/bus/include/bus/bus_driver.h b/hw/bus/include/bus/bus_driver.h
index 04306e5..934a6b8 100644
--- a/hw/bus/include/bus/bus_driver.h
+++ b/hw/bus/include/bus/bus_driver.h
@@ -22,6 +22,9 @@
 
 #include <stdint.h>
 #include "os/mynewt.h"
+#if MYNEWT_VAL(BUS_STATS)
+#include "stats/stats.h"
+#endif
 
 #ifdef __cplusplus
 extern "C" {
@@ -30,6 +33,16 @@ extern "C" {
 struct bus_dev;
 struct bus_node;
 
+#if MYNEWT_VAL(BUS_STATS)
+STATS_SECT_START(bus_stats_section)
+    STATS_SECT_ENTRY(lock_timeouts)
+    STATS_SECT_ENTRY(read_ops)
+    STATS_SECT_ENTRY(read_errors)
+    STATS_SECT_ENTRY(write_ops)
+    STATS_SECT_ENTRY(write_errors)
+STATS_SECT_END
+#endif
+
 /**
  * Bus device operations
  *
@@ -88,6 +101,10 @@ struct bus_dev {
     struct os_mutex lock;
     struct bus_node *configured_for;
 
+#if MYNEWT_VAL(BUS_STATS)
+    STATS_SECT_DECL(bus_stats_section) stats;
+#endif
+
 #if MYNEWT_VAL(BUS_DEBUG_OS_DEV)
     uint32_t devmagic;
 #endif
@@ -112,6 +129,10 @@ struct bus_node {
         void *init_arg;
     };
 
+#if MYNEWT_VAL(BUS_STATS_PER_NODE)
+    STATS_SECT_DECL(bus_stats_section) stats;
+#endif
+
 #if MYNEWT_VAL(BUS_DEBUG_OS_DEV)
     uint32_t nodemagic;
 #endif
diff --git a/hw/bus/src/bus.c b/hw/bus/src/bus.c
index 0bc7c0f..ab808ed 100644
--- a/hw/bus/src/bus.c
+++ b/hw/bus/src/bus.c
@@ -23,9 +23,39 @@
 #include "bus/bus.h"
 #include "bus/bus_debug.h"
 #include "bus/bus_driver.h"
+#if MYNEWT_VAL(BUS_STATS)
+#include "stats/stats.h"
+#endif
 
 static os_time_t g_bus_node_lock_timeout;
 
+#if MYNEWT_VAL(BUS_STATS)
+STATS_NAME_START(bus_stats_section)
+    STATS_NAME(bus_stats_section, lock_timeouts)
+    STATS_NAME(bus_stats_section, read_ops)
+    STATS_NAME(bus_stats_section, read_errors)
+    STATS_NAME(bus_stats_section, write_ops)
+    STATS_NAME(bus_stats_section, write_errors)
+STATS_NAME_END(bus_stats_section)
+
+#if MYNEWT_VAL(BUS_STATS_PER_NODE)
+#define BUS_STATS_INC(_bdev, _bnode, _var)  \
+    do {                                    \
+        STATS_INC((_bdev)->stats, _var);    \
+        STATS_INC((_bnode)->stats, _var);   \
+    } while (0)
+#else
+#define BUS_STATS_INC(_bdev, _bnode, _var)  \
+    do {                                    \
+        STATS_INC((_bdev)->stats, _var);    \
+    } while (0)
+#endif
+#else
+#define BUS_STATS_INC(_bdev, _bnode, _var)  \
+    do {                                    \
+    } while (0)
+#endif
+
 static int
 bus_node_open_func(struct os_dev *odev, uint32_t wait, void *arg)
 {
@@ -92,6 +122,9 @@ bus_dev_init_func(struct os_dev *odev, void *arg)
 {
     struct bus_dev *bdev = (struct bus_dev *)odev;
     struct bus_dev_ops *ops = arg;
+#if MYNEWT_VAL(BUS_STATS)
+    char *stats_name;
+#endif
 
     BUS_DEBUG_POISON_DEV(bdev);
 
@@ -100,6 +133,15 @@ bus_dev_init_func(struct os_dev *odev, void *arg)
 
     os_mutex_init(&bdev->lock);
 
+#if MYNEWT_VAL(BUS_STATS)
+    asprintf(&stats_name, "bd_%s", odev->od_name);
+    /* XXX should we assert or return error on failure? */
+    stats_init_and_reg(STATS_HDR(bdev->stats),
+                       STATS_SIZE_INIT_PARMS(bdev->stats, STATS_SIZE_32),
+                       STATS_NAME_INIT_PARMS(bus_stats_section),
+                       stats_name);
+#endif
+
     return 0;
 }
 
@@ -110,6 +152,9 @@ bus_node_init_func(struct os_dev *odev, void *arg)
     struct bus_node_cfg *node_cfg = arg;
     struct os_dev *parent_odev;
     void *init_arg;
+#if MYNEWT_VAL(BUS_STATS_PER_NODE)
+    char *stats_name;
+#endif
 
     parent_odev = os_dev_lookup(node_cfg->bus_name);
     if (!parent_odev) {
@@ -125,6 +170,15 @@ bus_node_init_func(struct os_dev *odev, void *arg)
     odev->od_handlers.od_open = bus_node_open_func;
     odev->od_handlers.od_close = bus_node_close_func;
 
+#if MYNEWT_VAL(BUS_STATS_PER_NODE)
+    asprintf(&stats_name, "bn_%s", odev->od_name);
+    /* XXX should we assert or return error on failure? */
+    stats_init_and_reg(STATS_HDR(bnode->stats),
+                       STATS_SIZE_INIT_PARMS(bnode->stats, STATS_SIZE_32),
+                       STATS_NAME_INIT_PARMS(bus_stats_section),
+                       stats_name);
+#endif
+
     if (bnode->callbacks.init) {
         bnode->callbacks.init(bnode, init_arg);
     }
@@ -152,7 +206,11 @@ bus_node_read(struct os_dev *node, void *buf, uint16_t length,
         return rc;
     }
 
+    BUS_STATS_INC(bdev, bnode, read_ops);
     rc = bdev->dops->read(bdev, bnode, buf, length, timeout, flags);
+    if (rc) {
+        BUS_STATS_INC(bdev, bnode, read_errors);
+    }
 
     (void)bus_node_unlock(node);
 
@@ -179,7 +237,11 @@ bus_node_write(struct os_dev *node, const void *buf, uint16_t length,
         return rc;
     }
 
+    BUS_STATS_INC(bdev, bnode, write_ops);
     rc = bdev->dops->write(bdev, bnode, buf, length, timeout, flags);
+    if (rc) {
+        BUS_STATS_INC(bdev, bnode, write_errors);
+    }
 
     (void)bus_node_unlock(node);
 
@@ -213,13 +275,17 @@ bus_node_write_read_transact(struct os_dev *node, const void *wbuf,
      * too many flags now (like we literally have only one flag) let's just pass
      * no flags for now
      */
+    BUS_STATS_INC(bdev, bnode, write_ops);
     rc = bdev->dops->write(bdev, bnode, wbuf, wlength, timeout, BUS_F_NOSTOP);
     if (rc) {
+        BUS_STATS_INC(bdev, bnode, write_errors);
         goto done;
     }
 
+    BUS_STATS_INC(bdev, bnode, read_ops);
     rc = bdev->dops->read(bdev, bnode, rbuf, rlength, timeout, flags);
     if (rc) {
+        BUS_STATS_INC(bdev, bnode, read_errors);
         goto done;
     }
 
@@ -247,6 +313,7 @@ bus_node_lock(struct os_dev *node, os_time_t timeout)
 
     err = os_mutex_pend(&bdev->lock, timeout);
     if (err == OS_TIMEOUT) {
+        BUS_STATS_INC(bdev, bnode, lock_timeouts);
         return SYS_ETIMEOUT;
     }
 
diff --git a/hw/bus/syscfg.yml b/hw/bus/syscfg.yml
index b7e03f3..db25199 100644
--- a/hw/bus/syscfg.yml
+++ b/hw/bus/syscfg.yml
@@ -32,6 +32,18 @@ syscfg.defs:
             transaction APIs (i.e. without timeout set explicitly)
         value: 50
 
+    BUS_STATS:
+        description: >
+            Enable statistics for bus devices. By default only global per-device
+            statistics are enabled. Use BUS_STATS_PER_NODE to enable statistics
+            for each node also.
+        value: 0
+    BUS_STATS_PER_NODE:
+        description: >
+            Enable per-node statistics for each bus node.
+        value: 0
+        restrictions: BUS_STATS
+
     BUS_DEBUG_OS_DEV:
         description: >
             Enable additional debugging for os_dev objects.