linux-kernel - Re: [PATCH] sched: fair: Use the earliest break even

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] [day] [month] [year] [list]

Date:   Fri, 6 Mar 2020 10:28:46 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Valentin Schneider <valentin.schneider@....com>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        "open list:SCHEDULER" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] sched: fair: Use the earliest break even


Hi Valentin,

[cc'ing Thomas Gleixner]

On 04/03/2020 19:31, Valentin Schneider wrote:
> 
> On Wed, Mar 04 2020, Daniel Lezcano wrote:
>>> With that said, that comment actually raises a valid point: picking
>>> recently idled CPUs might give us warmer cache. So by using the break
>>> even stat, we can end up picking CPUs with colder caches (have been
>>> idling for longer) than the current logic would. I suppose more testing
>>> will tell us where we stand.
>>
>> Actually I'm not sure this comment still applies. If the CPU is powered
>> down, the cache is flushed or we can be picking up CPU with their cache
>> totally trashed by interrupt processing.
>>
>>>> +++ b/kernel/sched/sched.h
>>>> @@ -1015,6 +1015,7 @@ struct rq {
>>>>  #ifdef CONFIG_CPU_IDLE
>>>>       /* Must be inspected within a rcu lock section */
>>>>       struct cpuidle_state	*idle_state;
>>>> +	s64			break_even;
>>>
>>> Why signed? This should be purely positive, right?
>>
>> It should be, but s64 complies with the functions ktime_to_ns signature.
>>
>> static inline s64 ktime_to_ns(const ktime_t kt)
>>
> 
> Would there be harm then in simply storing:
> 
>   ktime_get_ns() + idle_state->exit_latency_ns
> 
> (which is u64)?

Actually I'm not sure, ktime_get_ns() is correct, it returns an u64 but
it is actually returning:

	return ktime_to_ns(ktime_get());

where ktime_to_ns is returning a s64.

There are the following functions:

static inline u64 ktime_get_coarse_ns(void)
static inline u64 ktime_get_coarse_real_ns(void)
static inline u64 ktime_get_coarse_boottime_ns(void)
static inline u64 ktime_get_coarse_clocktai_ns(void)
static inline u64 ktime_get_ns(void)
static inline u64 ktime_get_real_ns(void)
static inline u64 ktime_get_boottime_ns(void)
static inline u64 ktime_get_clocktai_ns(void)
static inline u64 ktime_get_raw_ns(void)

extern u64 ktime_get_mono_fast_ns(void);
extern u64 ktime_get_raw_fast_ns(void);
extern u64 ktime_get_boot_fast_ns(void);
extern u64 ktime_get_real_fast_ns(void);

They are all based on ktime_get_ns() which returns a s64.

Not sure this type mismatch was done on purpose or not.

Thomas?



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog