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

[Stefan Roese <sr@denx.de>]

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