Hi Simon,
On 7/13/23 22:51, Simon Glass wrote:
Hi Stefan,
On Thu, 13 Jul 2023 at 08:17, Stefan Roese <s...@denx.de> wrote:
This patch adds the new Kconfig option CONFIG_CYCLIC_RATELIMIT_US which
defines the min allowed time after with a new call into the cyclic
infrastructure is allowed. This results in a ratelimiting of the all
functions hooked into the cyclic interface. As it's been noticed that on
some platforms, that high frequent calls to schedule() (replaced from
WATCHDOG_RESET) may lead to a performance degration.
When a high frequent calling into schedule() is detected, a warning is
logged only once to help indentify this frantic caller.
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Christophe Leroy <christophe.le...@csgroup.eu>
---
common/Kconfig | 11 +++++++++++
common/cyclic.c | 27 +++++++++++++++++++++++++--
include/asm-generic/global_data.h | 1 +
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig
index 42baca20a618..0611cce301a5 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -601,6 +601,17 @@ config CYCLIC_MAX_CPU_TIME_US
takes longer than this duration this function will get unregistered
automatically.
+config CYCLIC_RATELIMIT_US
+ int "Sets the min time/delay in us after a new call into schedule is
done"
+ default 100
1000 ?
1000 was my first pick as well. Leading to this WARN_ONE warning:
schedule() called very often, now = 1085562us next call at 1086380us
This is why I switched to a less strict rate limitation as default.
Other platforms may choose to override this value if this matches their
use cases.
+ help
+ The min allowed time after with a new call into the cyclic
+ infrastructure is allowed. This results in a ratelimiting of the
rate limiting
Thanks.
+ all functions hooked into the cyclic interface.
+ As it's been noticed that on some platforms, that high frequent
+ calls to schedule() (replaced from WATCHDOG_RESET) may lead to a
+ performance degration.
degradation
also below
Thanks, will update in v2.
+
endif # CYCLIC
config EVENT
diff --git a/common/cyclic.c b/common/cyclic.c
index a49bfc88f5c0..c945ae55c965 100644
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -12,6 +12,7 @@
#include <log.h>
#include <malloc.h>
#include <time.h>
+#include <linux/bug.h>
#include <linux/errno.h>
#include <linux/list.h>
#include <asm/global_data.h>
@@ -109,8 +110,30 @@ void schedule(void)
* schedule() might get called very early before the cyclic IF is
* ready. Make sure to only call cyclic_run() when it's initalized.
*/
- if (gd)
- cyclic_run();
+ if (gd) {
+ uint64_t now;
Should be u64 in here I think.
This file is already using uint64_t instead of u64. This is why I used
this type here as well.
+
+ /*
+ * Add some ratelimiting to not call into the cyclic IF too
+ * ofter. As it's been noticed that on some platforms, that
+ * high frequent calls to schedule() (replaced from
+ * WATCHDOG_RESET) may lead to a performance degration.
+ */
+ /* Do not reset the watchdog too often */
join with above comment
Ok, will do.
Thanks,
Stefan
+ now = timer_get_us();
+ if (time_after_eq64(now, gd->cyclic_next_call)) {
+ cyclic_run();
+ gd->cyclic_next_call = now + CONFIG_CYCLIC_RATELIMIT_US;
+ } else {
+ /*
+ * Throw a warning (only once) to help identifying
+ * frantic callers
+ */
+ WARN_ONCE(1, "schedule() called very often, now = %lld us
next call at %lld us, dt = %lld us\n",
+ now, gd->cyclic_next_call,
+ gd->cyclic_next_call - now);
+ }
+ }
}
int cyclic_unregister_all(void)
diff --git a/include/asm-generic/global_data.h
b/include/asm-generic/global_data.h
index a1e1b9d64005..af26ae4dfc65 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -484,6 +484,7 @@ struct global_data {
* @cyclic_list: list of registered cyclic functions
*/
struct hlist_head cyclic_list;
+ uint64_t cyclic_next_call;
#endif
/**
* @dmtag_list: List of DM tags
--
2.41.0
Regards,
Simon
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
