On 22/2/2022 1:52 PM, Miao Li wrote:
This patch adds a log in rte_power_monitor to show the core has been
waked up.

Signed-off-by: Miao Li <miao...@intel.com>
---
  lib/eal/x86/rte_power_intrinsics.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/lib/eal/x86/rte_power_intrinsics.c 
b/lib/eal/x86/rte_power_intrinsics.c
index f749da9b85..dd63e2b6eb 100644
--- a/lib/eal/x86/rte_power_intrinsics.c
+++ b/lib/eal/x86/rte_power_intrinsics.c
@@ -128,6 +128,14 @@ rte_power_monitor(const struct rte_power_monitor_cond *pmc,
                        : "D"(0), /* enter C0.2 */
                          "a"(tsc_l), "d"(tsc_h));
+ cur_value = __get_umwait_val(pmc->addr, pmc->size);
+
+       /* check if core has been waked up by changing monitoring value */
+       if (pmc->fn(cur_value, pmc->opaque) != 0)
+               RTE_LOG(INFO, EAL,
+                       "lcore %u is waked up from value change\n",
+                       rte_lcore_id());
+
  end:
        /* erase sleep address */
        rte_spinlock_lock(&s->lock);


Hi Li,

If I'm not mistaken, a similar patch was added to a previous DPDK release and then removed because of the enormous performance impact.

This looks to be something similar, and it's adding a log message to a low-level function. Also, as mentioned before, the intention in the future is to call this function much more agressively, so there would be hundreds of thousands of messages every second.

We cannot add an RTE_LOG here. Please rework and put the log in the test case instead.

Also, regarding the wording, I would suggest  "lcore %u awoke due to monitor address value change\n"

Rgds,

Dave.


Reply via email to