[PATCH 2/2] cyclic: parse cyclic list only if next cyclic timestamp is reached

[Marek Vasut]

On 7/3/26 11:55 AM, Patrice CHOTARD wrote:

Hello Patrice,

>>>    +    /* check if the next cyclic function's timestamp is reached */
>>> +    now = get_timer_us(0);
>>> +    if (time_after_eq64(gd->next_cyclic_call, now))
>>> +        return;
>>> +
>>>        gd->flags |= GD_FLG_CYCLIC_RUNNING;
>>>        hlist_for_each_entry_safe(cyclic, tmp, cyclic_get_list(), list) {
>>>            /*
>>> @@ -102,7 +108,13 @@ static void cyclic_run(void)
>>>                    cyclic->already_warned = true;
>>>                }
>>>            }
>>> +        if (next_call > cyclic->next_call)
>>> +            next_call = cyclic->next_call;
>>
>> Can you move this entry to the beginning of the cyclic list, so that the entry with the earliest deadline would be right at the beginning of the list ? That should squeeze some more improvement out of this I think.
> 
> Hi Marek
> 
> I did a test, it doesn't bring any noticeable improvements, but nevertheless i will submit a v3 with this update.

Does the list iteration exit when it finds the first entry that is not 
going to be executed at this time ? If it does not, then there will be 
no improvement. But for this to work, the list would have to be surely 
fully ordered in incrementing timestamp order, is it ?

>>
>>>        }
>>> +
>>> +    if (next_call != UINT64_MAX)
>>
>> Can the next_call literally hit UINT64_MAX in that one unlikely case ?
>> You likely do need a separate flag, bool next_call_valid, to avoid that edge case.
> 
> In case, no cyclic has been registered, we don't parse the cyclic list and next_call stays set to UINT64_MAX
> We don't want that next_cyclic_call be set to UINT64_MAX, otherwise in that case, we will always exit cyclic_run() before parsing cyclic list.
Consider this:

+	u64 next_call = UINT64_MAX;
...
  	hlist_for_each_entry_safe(cyclic, tmp, cyclic_get_list(), list) {
...
                 cyclic->next_call = now + cyclic->delay_us; // = ~0ULL
...
+		if (next_call > cyclic->next_call)      // False
+			next_call = cyclic->next_call;  // Not set
...
+	if (next_call != UINT64_MAX) // False, branch not executed, but it 
should be executed ?
+		gd->next_cyclic_call = next_call;
...