Hi, > -----Original Message----- > From: Hunt, David <david.h...@intel.com> > Sent: Wednesday, February 23, 2022 12:32 AM > To: Li, Miao <miao...@intel.com>; dev@dpdk.org > Cc: Wang, Yinan <yinan.w...@intel.com>; step...@networkplumber.org > Subject: Re: [PATCH v1] power: add wakeup log > > > 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.
I add a judgment before the output. When no packet arriver, the log will not be printed. When a lot of packets arriver, the rte_power_monitor will not be called. So I think the performance impact is small. > > Also, regarding the wording, I would suggest "lcore %u awoke due to > monitor address value change\n" I will change the log content in next version. > > Rgds, > > Dave. > Thanks, Miao
