linux-kernel - Re: [PATCH v5 4/6] sched/isolation: Force housekeeping if isolcpus and nohz

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: <685cdf5db59a8aaf0b7c0d9847d109d878da039b.camel@redhat.com>
Date: Tue, 20 May 2025 13:17:20 +0200
From: Gabriele Monaco <gmonaco@...hat.com>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, 
 Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v5 4/6] sched/isolation: Force housekeeping if isolcpus
 and nohz_full don't leave any



On Tue, 2025-05-20 at 12:17 +0200, Frederic Weisbecker wrote:
> Le Thu, May 08, 2025 at 04:53:24PM +0200, Gabriele Monaco a écrit :
> > Currently the user can set up isolcpus and nohz_full in such a way
> > that
> > leaves no housekeeping CPU (i.e. no CPU that is neither domain
> > isolated
> > nor nohz full). This can be a problem for other subsystems (e.g.
> > the
> > timer wheel imgration).
> > 
> > Prevent this configuration by setting the boot CPU as housekeeping
> > if
> > the union of isolcpus and nohz_full covers all CPUs. In a similar
> > fashion as it already happens if either of them covers all CPUs.
> > 
> > Signed-off-by: Gabriele Monaco <gmonaco@...hat.com>
> > ---
> >  include/linux/tick.h     |  2 ++
> >  kernel/sched/isolation.c | 20 ++++++++++++++++++++
> >  kernel/time/tick-sched.c |  7 +++++++
> >  3 files changed, 29 insertions(+)
> > 
> > diff --git a/include/linux/tick.h b/include/linux/tick.h
> > index b8ddc8e631a3..0b32c0bd3512 100644
> > --- a/include/linux/tick.h
> > +++ b/include/linux/tick.h
> > @@ -278,6 +278,7 @@ static inline void tick_dep_clear_signal(struct
> > signal_struct *signal,
> >  extern void tick_nohz_full_kick_cpu(int cpu);
> >  extern void __tick_nohz_task_switch(void);
> >  extern void __init tick_nohz_full_setup(cpumask_var_t cpumask);
> > +extern void __init tick_nohz_full_clear_cpu(unsigned int cpu);
> >  #else
> >  static inline bool tick_nohz_full_enabled(void) { return false; }
> >  static inline bool tick_nohz_full_cpu(int cpu) { return false; }
> > @@ -304,6 +305,7 @@ static inline void tick_dep_clear_signal(struct
> > signal_struct *signal,
> >  static inline void tick_nohz_full_kick_cpu(int cpu) { }
> >  static inline void __tick_nohz_task_switch(void) { }
> >  static inline void tick_nohz_full_setup(cpumask_var_t cpumask) { }
> > +static inline void tick_nohz_full_clear_cpu(unsigned int cpu) { }
> >  #endif
> >  
> >  static inline void tick_nohz_task_switch(void)
> > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> > index 81bc8b329ef1..27b65b401534 100644
> > --- a/kernel/sched/isolation.c
> > +++ b/kernel/sched/isolation.c
> > @@ -165,6 +165,26 @@ static int __init housekeeping_setup(char
> > *str, unsigned long flags)
> >  			}
> >  		}
> >  
> > +		/* Check in combination with the previously set
> > cpumask */
> > +		type = find_first_bit(&housekeeping.flags,
> > HK_TYPE_MAX);
> > +		first_cpu =
> > cpumask_first_and_and(cpu_present_mask,
> > +						 
> > housekeeping_staging,
> > +						 
> > housekeeping.cpumasks[type]);
> > +		if (first_cpu >= nr_cpu_ids || first_cpu >=
> > setup_max_cpus) {
> > +			pr_warn("Housekeeping: must include one
> > present CPU neither "
> > +				"in nohz_full= nor in isolcpus=,
> > using boot CPU:%d\n",
> > +				smp_processor_id());
> > +			for_each_set_bit(type,
> > &housekeeping.flags, HK_TYPE_MAX)
> > +				__cpumask_set_cpu(smp_processor_id
> > (),
> > +						 
> > housekeeping.cpumasks[type]);
> > +			__cpumask_set_cpu(smp_processor_id(),
> > housekeeping_staging);
> > +			__cpumask_clear_cpu(smp_processor_id(),
> > non_housekeeping_mask);
> > +			tick_nohz_full_clear_cpu(smp_processor_id(
> > ));
> > +
> > +			if (cpumask_empty(non_housekeeping_mask))
> > +				goto free_housekeeping_staging;
> > +		}
> > +
> 
> Looking again at that, how is it possible to set a different CPU
> between
> isolcpus= and nohz_full= ?
> 
> 		enum hk_type type;
> 		unsigned long iter_flags = flags &
> housekeeping.flags;
> 
> 		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX) {
> 			if (!cpumask_equal(housekeeping_staging,
> 					  
> housekeeping.cpumasks[type])) {
> 				pr_warn("Housekeeping: nohz_full=
> must match isolcpus=\n");
> 				goto free_housekeeping_staging;
> 			}
> 		}

The isolcpus parameter can be used like:
1. isolcpus=1,2,3
2. isolcpus=domain,1,2,3
3. isolcpus=nohz,1,2,3
4. isolcpus=domain,nohz,1,2,3
...

1 and 2 are equivalent (e.g. if no mode is specified, that's domain
isolation), 3 is equivalent to nohz_full=1,2,3 and 4 is equivalent to
1-2 in combination with nohz_full=1,2,3

Now, the code takes into account that there are 2 arguments that can
isolate (isolcpus and domain) and can be passed in any order, that
specific code guards against those two passing inconsistent maps, e.g.:

	isolcpus=nohz,0-4 nohz_full=5-8

Strictly speaking it's guarding for any other possible inconsistency
but I believe that's the only one actually achievable.

Again, nothing forbids e.g.

	isolcpus=domain,0-4 nohz_full=5-8

since they're different isolation flags and that's allowed (not sure if
it really should be though).