Question on expiring HRtimer in-kernel

Question on expiring HRtimer in-kernel

[Linus Walleij]

I have a problem with a premature expiring HRtimer.

The HRtimer hrtimer_set_expires_range() is used in two places in
the upstream kernel:
kernel/futex/core.c
drivers/power/supply/ab8500_chargalg.c

Now I am testing the code in the latter, and it has seen some
bitrot since merged in 2012. Maybe it was correct at one point.
The timer is started like this:

    hrtimer_init(&di->safety_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
(...)
    hrtimer_set_expires_range(&di->safety_timer,
        ktime_set(timer_expiration * ONE_HOUR_IN_SECONDS, 0),
        ktime_set(FIVE_MINUTES_IN_SECONDS, 0));
    hrtimer_start_expires(&di->safety_timer, HRTIMER_MODE_REL);

What the author wanted to achieve is a very definitive callback in one
hour relative to now +/- 5 min, and that is one hour later in the
physical world,
as this deals with battery charging.

However sometimes this fires almost immediately rather than in an hour.

My first thought is to pass HRTIMER_MODE_REL also to init as
hrtimer_set_expires_range() could make things happen immediately
if we have ABS set, but this is all just intuitive.

Any hints? Better ways to create a definitive event in one hour?

Yours,
Linus Walleij

Re: Question on expiring HRtimer in-kernel

[Linus Walleij]

Sorry for top posting, the helpful autocomplete in gmail think Rohm Europe
is better to address than linux-pm@vger...

On Tue, Mar 1, 2022 at 1:19 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> I have a problem with a premature expiring HRtimer.
>
> The HRtimer hrtimer_set_expires_range() is used in two places in
> the upstream kernel:
> kernel/futex/core.c
> drivers/power/supply/ab8500_chargalg.c
>
> Now I am testing the code in the latter, and it has seen some
> bitrot since merged in 2012. Maybe it was correct at one point.
> The timer is started like this:
>
>     hrtimer_init(&di->safety_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
> (...)
>     hrtimer_set_expires_range(&di->safety_timer,
>         ktime_set(timer_expiration * ONE_HOUR_IN_SECONDS, 0),
>         ktime_set(FIVE_MINUTES_IN_SECONDS, 0));
>     hrtimer_start_expires(&di->safety_timer, HRTIMER_MODE_REL);
>
> What the author wanted to achieve is a very definitive callback in one
> hour relative to now +/- 5 min, and that is one hour later in the
> physical world,
> as this deals with battery charging.
>
> However sometimes this fires almost immediately rather than in an hour.
>
> My first thought is to pass HRTIMER_MODE_REL also to init as
> hrtimer_set_expires_range() could make things happen immediately
> if we have ABS set, but this is all just intuitive.
>
> Any hints? Better ways to create a definitive event in one hour?
>
> Yours,
> Linus Walleij

Re: Question on expiring HRtimer in-kernel

[Matti Vaittinen]

Hi Linus,

On 3/1/22 14:19, Linus Walleij wrote:
> I have a problem with a premature expiring HRtimer.
> 
> The HRtimer hrtimer_set_expires_range() is used in two places in
> the upstream kernel:
> kernel/futex/core.c
> drivers/power/supply/ab8500_chargalg.c
> 
> Now I am testing the code in the latter, and it has seen some
> bitrot since merged in 2012. Maybe it was correct at one point.
> The timer is started like this:
> 
>      hrtimer_init(&di->safety_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
> (...)
>      hrtimer_set_expires_range(&di->safety_timer,
>          ktime_set(timer_expiration * ONE_HOUR_IN_SECONDS, 0),
>          ktime_set(FIVE_MINUTES_IN_SECONDS, 0));
>      hrtimer_start_expires(&di->safety_timer, HRTIMER_MODE_REL);
> 
> What the author wanted to achieve is a very definitive callback in one
> hour relative to now +/- 5 min, and that is one hour later in the
> physical world,
> as this deals with battery charging.
> 
> However sometimes this fires almost immediately rather than in an hour.
> 
> My first thought is to pass HRTIMER_MODE_REL also to init as
> hrtimer_set_expires_range() could make things happen immediately
> if we have ABS set, but this is all just intuitive.
> 
> Any hints? Better ways to create a definitive event in one hour?
> 

_a lot_ of water has been flowing in the Oulu river since I last touched 
on any code like this. Unfortunately I can't go back to my old code as 
it was left in proprietary world. So no promises I am at all helpful here ;)

In any case, I remember few cases where I hit nasty issues because I 
used CLOCK_REALTIME - which (AFAIR) is subject to the time adjustments. 
NTP, GPS-time and so on can make the time tick in a strange way :) I 
guess you would have noticed if time was set when timer did expire.

Anyways, I guess the battery charging should rather be tied CLOCK_MONOTONIC.

No guarantees though - please ignore me if I make no sense. Oh, 
actually, please don't - please correct me instead :)


-- Matti

Re: Question on expiring HRtimer in-kernel

[Linus Walleij]

On Thu, Mar 3, 2022 at 1:43 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> In any case, I remember few cases where I hit nasty issues because I
> used CLOCK_REALTIME - which (AFAIR) is subject to the time adjustments.
> NTP, GPS-time and so on can make the time tick in a strange way :) I
> guess you would have noticed if time was set when timer did expire.
>
> Anyways, I guess the battery charging should rather be tied CLOCK_MONOTONIC.

That makes a lot of sense, and is what I have learned from the internal
kernel primitives as well, I just assumed the HRTimer was some special
kind of beast.

I'll patch this and run some testing, if it goes away with MONOTONIC
I'll send a patch. Thanks!

Yours,
Linus Walleij