linux-kernel - Re: [PATCH] tick: check if broadcast device could really be stopped

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: <c3565734-05e3-0a9d-1101-92c4be476ae6@st.com>
Date:   Mon, 14 Oct 2019 15:22:35 +0000
From:   Benjamin GAIGNARD <benjamin.gaignard@...com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     "fweisbec@...il.com" <fweisbec@...il.com>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "marc.zyngier@....com" <marc.zyngier@....com>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH] tick: check if broadcast device could really be stopped


On 10/14/19 4:28 PM, Thomas Gleixner wrote:
> On Mon, 14 Oct 2019, Benjamin GAIGNARD wrote:
>> On 10/14/19 3:40 PM, Thomas Gleixner wrote:
>>> I don't understand what you are trying to achieve here. If the tick device,
>>> i.e. the comparator stops working in deep idle states, then the device has
>>> rightfully the CLOCK_EVT_FEAT_C3STOP (mis)feature set. Which means that the
>>> system needs a fallback device for the deep idle case. If there is no
>>> physical fallback device then you should enable the hrtimer based broadcast
>>> pseudo tick device.
>>>
>>> If the CPUs never go deep idle because there is no idle driver loaded or
>>> the deep power state in which the comparator stops working is never
>>> reached, then the broadcast hrtimer will never be used. It just eats a bit
>>> of memory and a few extra instructions.
>> Since CPUs won't go in deep idle I don't want to get a broadcast timer
>> but only an hrtimer to get accure latencies for the scheduler.
> What's wrong with the broadcast timer if it is never utilized? It's there,
> needs a few bytes of memory and that's it.
>
>> When arch arm timer doesn't set CLOCK_EVT_FEAT_C3STOP flag, my system got
>> a hrtimer and everything goes well.
> Sure, but that's applicable to your particular system only and not a
> generic solution. Neither your DT hack, nor the other one you posted.
>
>> If arch arm timer set CLOCK_EVT_FEAT_C3STOP hrtimer disappear (except if
>> I add an additional broadcast timer).
> Right.
>
>> What is the link between tick broadcast timer and hrtimer ? Does arch
>> arm timer could only implement them at the same time ?
> If the clock event device is affected by deep power states, then the core
> requires a fallback device, aka. broadcast timer, which makes sure that no
> event is lost in case that the CPU goes into a deep power state. If no CPU
> ever goes deep, the broadcast timer is there and unused.
>
> Obviously the system cannot enable high resolution timers if the clock
> event device is affected by power states.
>
> Unless you have a solution which works under all circumstances (and the
> current patch definitely does not) then you have to deal with the
> requirement of the broadcast timer (either physical or software based).

If I follow you I need, for my system, a software based broadcast timer 
(tick-broadcast-hrtimer ?).

If that is correct I 'just' need to add a call to 
tick_setup_hrtimer_broadcast() in arch/arm/kernel/time.c

Regards,

Benjamin

>
> "I don't want a broadcast timer falls" into the "I want a pony" realm.
>
> Thanks,
>
> 	tglx
>