The function cpu_hotplug_not_supported() can be called to indicate that
CPU hotplug should be disabled. It does not prevent the initial bring up
of the CPU, but it stops subsequent offlining.
This function is intended to replace CC_ATTR_HOTPLUG_DISABLED.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
include/linux/cpu.h | 2 ++
kernel/cpu.c | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index f19f56501809..aab3887cadbc 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -132,6 +132,7 @@ extern void cpus_read_lock(void);
extern void cpus_read_unlock(void);
extern int cpus_read_trylock(void);
extern void lockdep_assert_cpus_held(void);
+extern void cpu_hotplug_not_supported(void);
extern void cpu_hotplug_disable(void);
extern void cpu_hotplug_enable(void);
void clear_tasks_mm_cpumask(int cpu);
@@ -147,6 +148,7 @@ static inline void cpus_read_lock(void) { }
static inline void cpus_read_unlock(void) { }
static inline int cpus_read_trylock(void) { return true; }
static inline void lockdep_assert_cpus_held(void) { }
+static inline void cpu_hotplug_not_supported(void) { }
static inline void cpu_hotplug_disable(void) { }
static inline void cpu_hotplug_enable(void) { }
static inline int remove_cpu(unsigned int cpu) { return -EPERM; }
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6de7c6bb74ee..cf536fe1a88a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -484,6 +484,9 @@ static int cpu_hotplug_disabled;
DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock);
+/* Cleared if platform declares CPU hotplug not supported */
+static bool cpu_hotplug_supported = true;
+
void cpus_read_lock(void)
{
percpu_down_read(&cpu_hotplug_lock);
@@ -543,6 +546,18 @@ static void lockdep_release_cpus_lock(void)
rwsem_release(&cpu_hotplug_lock.dep_map, _THIS_IP_);
}
+/*
+ * Declare CPU hotplug not supported.
+ *
+ * It doesn't prevent initial bring up of the CPU, but stops offlining.
+ */
+void cpu_hotplug_not_supported(void)
+{
+ cpu_maps_update_begin();
+ cpu_hotplug_supported = false;
+ cpu_maps_update_done();
+}
+
/*
* Wait for currently running CPU hotplug operations to complete (if any) and
* disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects
@@ -1507,7 +1522,7 @@ static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
* If the platform does not support hotplug, report it explicitly to
* differentiate it from a transient offlining failure.
*/
- if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED))
+ if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED) || !cpu_hotplug_supported)
return -EOPNOTSUPP;
if (cpu_hotplug_disabled)
return -EBUSY;
--
2.41.0
On 10/5/2023 6:13 AM, Kirill A. Shutemov wrote:
> The function cpu_hotplug_not_supported() can be called to indicate that
> CPU hotplug should be disabled. It does not prevent the initial bring up
> of the CPU, but it stops subsequent offlining.
>
> This function is intended to replace CC_ATTR_HOTPLUG_DISABLED.
>
Looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> ---
> include/linux/cpu.h | 2 ++
> kernel/cpu.c | 17 ++++++++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index f19f56501809..aab3887cadbc 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -132,6 +132,7 @@ extern void cpus_read_lock(void);
> extern void cpus_read_unlock(void);
> extern int cpus_read_trylock(void);
> extern void lockdep_assert_cpus_held(void);
> +extern void cpu_hotplug_not_supported(void);
> extern void cpu_hotplug_disable(void);
> extern void cpu_hotplug_enable(void);
> void clear_tasks_mm_cpumask(int cpu);
> @@ -147,6 +148,7 @@ static inline void cpus_read_lock(void) { }
> static inline void cpus_read_unlock(void) { }
> static inline int cpus_read_trylock(void) { return true; }
> static inline void lockdep_assert_cpus_held(void) { }
> +static inline void cpu_hotplug_not_supported(void) { }
> static inline void cpu_hotplug_disable(void) { }
> static inline void cpu_hotplug_enable(void) { }
> static inline int remove_cpu(unsigned int cpu) { return -EPERM; }
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6de7c6bb74ee..cf536fe1a88a 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -484,6 +484,9 @@ static int cpu_hotplug_disabled;
>
> DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock);
>
> +/* Cleared if platform declares CPU hotplug not supported */
> +static bool cpu_hotplug_supported = true;
> +
> void cpus_read_lock(void)
> {
> percpu_down_read(&cpu_hotplug_lock);
> @@ -543,6 +546,18 @@ static void lockdep_release_cpus_lock(void)
> rwsem_release(&cpu_hotplug_lock.dep_map, _THIS_IP_);
> }
>
> +/*
> + * Declare CPU hotplug not supported.
> + *
> + * It doesn't prevent initial bring up of the CPU, but stops offlining.
> + */
> +void cpu_hotplug_not_supported(void)
> +{
> + cpu_maps_update_begin();
> + cpu_hotplug_supported = false;
> + cpu_maps_update_done();
> +}
Since this function is not used in this patch, do you need to add __maybe_unused to
avoid warnings?
> +
> /*
> * Wait for currently running CPU hotplug operations to complete (if any) and
> * disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects
> @@ -1507,7 +1522,7 @@ static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
> * If the platform does not support hotplug, report it explicitly to
> * differentiate it from a transient offlining failure.
> */
> - if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED))
> + if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED) || !cpu_hotplug_supported)
> return -EOPNOTSUPP;
> if (cpu_hotplug_disabled)
> return -EBUSY;
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On Tue, Oct 10, 2023 at 06:35:59AM -0700, Kuppuswamy Sathyanarayanan wrote:
>
>
> On 10/5/2023 6:13 AM, Kirill A. Shutemov wrote:
> > The function cpu_hotplug_not_supported() can be called to indicate that
> > CPU hotplug should be disabled. It does not prevent the initial bring up
> > of the CPU, but it stops subsequent offlining.
> >
> > This function is intended to replace CC_ATTR_HOTPLUG_DISABLED.
> >
>
> Looks good to me.
>
> Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
Thanks.
> > @@ -543,6 +546,18 @@ static void lockdep_release_cpus_lock(void)
> > rwsem_release(&cpu_hotplug_lock.dep_map, _THIS_IP_);
> > }
> >
> > +/*
> > + * Declare CPU hotplug not supported.
> > + *
> > + * It doesn't prevent initial bring up of the CPU, but stops offlining.
> > + */
> > +void cpu_hotplug_not_supported(void)
> > +{
> > + cpu_maps_update_begin();
> > + cpu_hotplug_supported = false;
> > + cpu_maps_update_done();
> > +}
>
> Since this function is not used in this patch, do you need to add __maybe_unused to
> avoid warnings?
Hm? I don't think compiler complains about non-static unused functions. It
has no visibility if it is used.
--
Kiryl Shutsemau / Kirill A. Shutemov
On Thu, Oct 05 2023 at 16:13, Kirill A. Shutemov wrote:
> The function cpu_hotplug_not_supported() can be called to indicate that
> CPU hotplug should be disabled. It does not prevent the initial bring up
> of the CPU, but it stops subsequent offlining.
This tells me what the patch is doing, but not the why.
> This function is intended to replace CC_ATTR_HOTPLUG_DISABLED.
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -132,6 +132,7 @@ extern void cpus_read_lock(void);
> extern void cpus_read_unlock(void);
> extern int cpus_read_trylock(void);
> extern void lockdep_assert_cpus_held(void);
> +extern void cpu_hotplug_not_supported(void);
This function name sucks.
The point is as you explained to prevent offlining, but not onlining. So
can we please make this very clear? Something like:
cpu_hotplug_disable_offlining()
> +/* Cleared if platform declares CPU hotplug not supported */
> +static bool cpu_hotplug_supported = true;
Again. This is not about disabling hotplug all together. Something like:
static bool cpu_hotplug_offline_disabled;
Which expresses clearly what this is about and does not require this
awkward negation.
Thanks,
tglx