linux-kernel - Re: [PATCH 1/2] sched/isolation: Add cpu_is

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]

Date:   Fri, 17 Mar 2023 15:33:13 -0300
From:   Marcelo Tosatti <mtosatti@...hat.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Leonardo Bras <leobras@...hat.com>,
        Frederic Weisbecker <fweisbecker@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Johannes Weiner <hannes@...xchg.org>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Shakeel Butt <shakeelb@...gle.com>,
        Muchun Song <muchun.song@...ux.dev>,
        LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org,
        Frederic Weisbecker <frederic@...nel.org>,
        Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH 1/2] sched/isolation: Add cpu_is_isolated() API

On Fri, Mar 17, 2023 at 02:44:47PM +0100, Michal Hocko wrote:
> From: Frederic Weisbecker <frederic@...nel.org>
> 
> Provide this new API to check if a CPU has been isolated either through
> isolcpus= or nohz_full= kernel parameter.
> 
> It aims at avoiding kernel load deemed to be safely spared on CPUs
> running sensitive workload that can't bear any disturbance, such as
> pcp cache draining.

Hi Michal,

This makes no sense to me.

HK_TYPE_DOMAIN is set when isolcpus=domain is configured.
HK_TYPE_TICK is set when nohz_full= is configured.

The use-cases i am aware of use either:

isolcpus=managed_irq,... nohz_full=
OR
isolcpus=domain,managed_irq,... nohz_full=

So what is the point of this function again?

Perhaps it made sense along with, but now does not make sense
anymore:

Subject: [PATCH 1/2] sched/isolation: Merge individual nohz_full features into a common housekeeping flag

The individual isolation features turned on by nohz_full were initially
split in order for each of them to be tunable through cpusets. However
plans have changed in favour of an interface (be it cpusets or sysctl)
grouping all these features to be turned on/off altogether. Then should
the need ever arise, the interface can still be expanded to handle the
individual isolation features.

But Michal can just use housekeeping_test_cpu(cpu, HK_TYPE_TICK) and
the convertion of nohz_full features into a common housekeeping flag
can convert that to something else later?



> 
> Suggested-by: Michal Hocko <mhocko@...e.com>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> Signed-off-by: Michal Hocko <mhocko@...e.com>
> ---
>  include/linux/sched/isolation.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/sched/isolation.h b/include/linux/sched/isolation.h
> index 8c15abd67aed..fe1a46f30d24 100644
> --- a/include/linux/sched/isolation.h
> +++ b/include/linux/sched/isolation.h
> @@ -46,6 +46,12 @@ static inline bool housekeeping_enabled(enum hk_type type)
>  
>  static inline void housekeeping_affine(struct task_struct *t,
>  				       enum hk_type type) { }
> +
> +static inline bool housekeeping_test_cpu(int cpu, enum hk_type type)
> +{
> +	return true;
> +}
> +
>  static inline void housekeeping_init(void) { }
>  #endif /* CONFIG_CPU_ISOLATION */
>  
> @@ -58,4 +64,10 @@ static inline bool housekeeping_cpu(int cpu, enum hk_type type)
>  	return true;
>  }
>  
> +static inline bool cpu_is_isolated(int cpu)
> +{
> +	return !housekeeping_test_cpu(cpu, HK_TYPE_DOMAIN) ||
> +		 !housekeeping_test_cpu(cpu, HK_TYPE_TICK);
> +}
> +
>  #endif /* _LINUX_SCHED_ISOLATION_H */
> -- 
> 2.30.2
> 
>