linux-kernel - Re: clock event device’s next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1710240917020.2369@nanos>
Date:   Tue, 24 Oct 2017 09:23:08 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Sodagudi Prasad <psodagud@...eaurora.org>
cc:     viresh.kumar@...aro.org, fweisbec@...il.com, mingo@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: clock event device’s next_event

On Mon, 23 Oct 2017, Sodagudi Prasad wrote:

> Hi Viresh and Thomas,
> 
> In the functions tick_nohz_stop_sched_tick(), when expires = KTIME_MAX we are
> canceling the tick_sched_timer timer but we are not updating the clock event
> device’s next_event to KTIME_MAX.
> Due to that broadcast device’s next_event is not programmed properly and
> resulting unnecessary wakeups for this cpu.
> 
>         /*
>          * If the expiration time == KTIME_MAX, then we simply stop
>          * the tick timer.
>          */
>         if (unlikely(expires == KTIME_MAX)) {
>                 if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
>                         hrtimer_cancel(&ts->sched_timer);
>                 goto out;
>         }

Right, because this code does not have access to the broadcast device at
all. It doesn't even know and care about it.

> After digging further, I see that following call flow is updating
> tick_cpu_device state to shutdown state but clock event device  next_event is
> not updated to KTIME_MAX.
> hrtimer_cancel -> __remove_hrtimer -> hrtimer_force_reprogram ->
> tick_program_event.
> 
> int tick_program_event(ktime_t expires, int force)
> {
>         struct clock_event_device *dev =
> __this_cpu_read(tick_cpu_device.evtdev);
> 
>         if (unlikely(expires == KTIME_MAX)) {
>                 /*
>                  * We don't need the clock event device any more, stop it.
>                  */
>                 clockevents_switch_state(dev,
> CLOCK_EVT_STATE_ONESHOT_STOPPED);
>                 return 0;
>         }
> In the above tick_program_event() function clock event device’s next_event is
> not getting updated as clockevents_program_event() function not called after
> state update.

If the device is shutdown, then next_event does not matter. But yes, for
consistency reasons we could set it to KTIME_MAX.

Thanks,

	tglx