[PATCH] cyclic: Don't disable cylic function upon exceeding CPU time

[PATCH] cyclic: Don't disable cylic function upon exceeding CPU time

[Stefan Roese]

With the migration of the watchdog infrastructure to cyclic functions
it's been noticed, that at least one watchdog driver is broken now. As
the execution time of it's watchdog reset function is quite long.

In general it's not really necessary (right now) to disable the cyclic
function upon exceeding CPU time usage. So instead of disabling the
cylic function in this case, let's just print a warning once to show
this potential problem to the user.

Signed-off-by: Stefan Roese <sr@denx.de>
Suggested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: Tom Rini <trini@konsulko.com>
Cc: Pali Rohár <pali@kernel.org>
---
 common/cyclic.c  | 12 ++++++++----
 include/cyclic.h |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/common/cyclic.c b/common/cyclic.c
index b3c180bd1a62..7abb82c16a90 100644
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -85,13 +85,17 @@ void cyclic_run(void)
 			cyclic->cpu_time_us += cpu_time;
 
 			/* Check if cpu-time exceeds max allowed time */
-			if (cpu_time > CONFIG_CYCLIC_MAX_CPU_TIME_US) {
-				pr_err("cyclic function %s took too long: %lldus vs %dus max, disabling\n",
+			if ((cpu_time > CONFIG_CYCLIC_MAX_CPU_TIME_US) &&
+			    (!cyclic->already_warned)) {
+				pr_err("cyclic function %s took too long: %lldus vs %dus max\n",
 				       cyclic->name, cpu_time,
 				       CONFIG_CYCLIC_MAX_CPU_TIME_US);
 
-				/* Unregister this cyclic function */
-				cyclic_unregister(cyclic);
+				/*
+				 * Don't disable this function, just warn once
+				 * about this exceeding CPU time usage
+				 */
+				cyclic->already_warned = true;
 			}
 		}
 	}
diff --git a/include/cyclic.h b/include/cyclic.h
index 760163643345..9c5c4fcc5468 100644
--- a/include/cyclic.h
+++ b/include/cyclic.h
@@ -39,6 +39,7 @@ struct cyclic_drv {
  * @run_cnt: Counter of executions occurances
  * @next_call: Next time in us, when the function shall be executed again
  * @list: List node
+ * @already_warned: Flag that we've warned about exceeding CPU time usage
  */
 struct cyclic_info {
 	void (*func)(void *ctx);
@@ -50,6 +51,7 @@ struct cyclic_info {
 	uint64_t run_cnt;
 	uint64_t next_call;
 	struct list_head list;
+	bool already_warned;
 };
 
 /** Function type for cyclic functions */
-- 
2.38.0

Re: [PATCH] cyclic: Don't disable cylic function upon exceeding CPU time

[Stefan Roese]

On 17.10.22 09:00, Stefan Roese wrote:
> With the migration of the watchdog infrastructure to cyclic functions
> it's been noticed, that at least one watchdog driver is broken now. As
> the execution time of it's watchdog reset function is quite long.
> 
> In general it's not really necessary (right now) to disable the cyclic
> function upon exceeding CPU time usage. So instead of disabling the
> cylic function in this case, let's just print a warning once to show
> this potential problem to the user.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Suggested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Pali Rohár <pali@kernel.org>

Applied to u-boot-watchdog/master

Thanks,
Stefan

> ---
>   common/cyclic.c  | 12 ++++++++----
>   include/cyclic.h |  2 ++
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/common/cyclic.c b/common/cyclic.c
> index b3c180bd1a62..7abb82c16a90 100644
> --- a/common/cyclic.c
> +++ b/common/cyclic.c
> @@ -85,13 +85,17 @@ void cyclic_run(void)
>   			cyclic->cpu_time_us += cpu_time;
>   
>   			/* Check if cpu-time exceeds max allowed time */
> -			if (cpu_time > CONFIG_CYCLIC_MAX_CPU_TIME_US) {
> -				pr_err("cyclic function %s took too long: %lldus vs %dus max, disabling\n",
> +			if ((cpu_time > CONFIG_CYCLIC_MAX_CPU_TIME_US) &&
> +			    (!cyclic->already_warned)) {
> +				pr_err("cyclic function %s took too long: %lldus vs %dus max\n",
>   				       cyclic->name, cpu_time,
>   				       CONFIG_CYCLIC_MAX_CPU_TIME_US);
>   
> -				/* Unregister this cyclic function */
> -				cyclic_unregister(cyclic);
> +				/*
> +				 * Don't disable this function, just warn once
> +				 * about this exceeding CPU time usage
> +				 */
> +				cyclic->already_warned = true;
>   			}
>   		}
>   	}
> diff --git a/include/cyclic.h b/include/cyclic.h
> index 760163643345..9c5c4fcc5468 100644
> --- a/include/cyclic.h
> +++ b/include/cyclic.h
> @@ -39,6 +39,7 @@ struct cyclic_drv {
>    * @run_cnt: Counter of executions occurances
>    * @next_call: Next time in us, when the function shall be executed again
>    * @list: List node
> + * @already_warned: Flag that we've warned about exceeding CPU time usage
>    */
>   struct cyclic_info {
>   	void (*func)(void *ctx);
> @@ -50,6 +51,7 @@ struct cyclic_info {
>   	uint64_t run_cnt;
>   	uint64_t next_call;
>   	struct list_head list;
> +	bool already_warned;
>   };
>   
>   /** Function type for cyclic functions */

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de