[RFC PATCH] watchdog: base rate-limiting on get_ticks() rather than get_timer()

[Stefan Roese]

On 05.06.20 16:18, Rasmus Villemoes wrote:
> On 05/06/2020 15.37, Stefan Roese wrote:
>> On 05.06.20 14:13, Stefan Roese wrote:
>>> On 05.06.20 14:11, Rasmus Villemoes wrote:
>>>> On 05/06/2020 13.48, Stefan Roese wrote:
>>>>> On 05.06.20 13:16, Rasmus Villemoes wrote:
>>>>>> This is what I had in mind. I also considered making it a config knob
>>>>>> (possibly just auto-selected based on $ARCH) whether to use
>>>>>> get_timer() or get_ticks(), but that becomes quite ugly.
>>>>>
>>>>> I hesitate a bit, moving with this generic code from get_timer()
>>>>> to get_ticks() for all boards. Did you test this patch on other
>>>>> platforms, like some ARM boards?
>>>>>
>>>>> Please note that I don't reject it - just asking.
>>>>
>>>> Yeah, I'm not really too happy about it myself, exactly because it
>>>> affects all arches/platforms. And no, I don't have other hardware handy
>>>> unfortunately. So it's very much an RFC where I hope someone with
>>>> knowledge of the various arches can say whether one can expect
>>>> get_ticks() to be at least as widely available as get_timer().
>>>
>>> I'll try to test it on a non-powerpc platform soon'ish.
>>
>> I've tested it on an MIPS based platform (gardena-smart-gateway-mt7688)
>> and it works there without any issues AFAICT.
> 
> Thanks.
> 
>> Two more remarks:
>>
>> Could you please run a build test with this patch applied for all
>> boards (Travis, Azure...)?
> 
> I may need a few more pointers than that. What am I supposed to do exactly?

Not sure if there is some documentation on how to use the Travis,
Gitlab or Azure build. It mainly uses buildman [1] internally, so you
can use buildman locally if you don't want to push the build into
the cloud.

>> And do you see an issue, that the timer-wrap fixed by Chris with this
>> patch [1] will not re-occur with your "ticks" approach?
> 
> No, it will not re-occur, because this does the comparison the right
> way, namely by subtracting the absolute times and comparing that delta
> to the threshold (that's also what the time_after does under the hood).
> The wrong way is saying now >= last_reset + threshold (or as I think the
> code used to do, computing next_reset = now + threshold and then asking
> now >= next_reset).

Ah, good to know. Thanks for confirming.

Thanks,
Stefan

[1] tools/buildman/README