linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu)
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-19 22:31   ` Thomas Gleixner
  2019-10-30 15:38 ` [PATCH 02/12] x86: Replace cpu_up/down with devcie_online/offline Qais Yousef
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Catalin Marinas, Will Deacon, Steve Capper,
	Richard Fontana, James Morse, Mark Rutland, Josh Poimboeuf,
	Ingo Molnar, Peter Zijlstra (Intel),
	Nicholas Piggin, Daniel Lezcano, Jiri Kosina, Pavankumar Kondeti,
	Zhenzhong Duan, linux-arm-kernel, linux-kernel

In preparation to make cpu_up/down private - move the user in arm64
hibernate.c to use a new generic function that provides what arm64
needs.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will@kernel.org>
CC: Steve Capper <steve.capper@arm.com>
CC: Richard Fontana <rfontana@redhat.com>
CC: James Morse <james.morse@arm.com>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Josh Poimboeuf <jpoimboe@redhat.com>
CC: Ingo Molnar <mingo@kernel.org>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: Nicholas Piggin <npiggin@gmail.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: Jiri Kosina <jkosina@suse.cz>
CC: Pavankumar Kondeti <pkondeti@codeaurora.org>
CC: Zhenzhong Duan <zhenzhong.duan@oracle.com>
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---

AFAICT we can't use device_online() directly here because suspend happens via
cpu_down() not device_offline(). If it is actually safe to use device_online()
then that would be simpler than creating the new function. Although the
operation seems generic enough to me and could benefit another arch user in the
future so the new function makes sense.


 arch/arm64/kernel/hibernate.c | 13 +++++--------
 include/linux/cpu.h           |  1 +
 kernel/cpu.c                  | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index e0a7fce0e01c..3b178055022f 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -166,14 +166,11 @@ int arch_hibernation_header_restore(void *addr)
 		sleep_cpu = -EINVAL;
 		return -EINVAL;
 	}
-	if (!cpu_online(sleep_cpu)) {
-		pr_info("Hibernated on a CPU that is offline! Bringing CPU up.\n");
-		ret = cpu_up(sleep_cpu);
-		if (ret) {
-			pr_err("Failed to bring hibernate-CPU up!\n");
-			sleep_cpu = -EINVAL;
-			return ret;
-		}
+
+	ret = hibernation_bringup_sleep_cpu(sleep_cpu);
+	if (ret) {
+		sleep_cpu = -EINVAL;
+		return ret;
 	}
 
 	resume_hdr = *hdr;
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 88dc0c653925..3b1fbe192989 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -87,6 +87,7 @@ int cpu_up(unsigned int cpu);
 void notify_cpu_starting(unsigned int cpu);
 extern void cpu_maps_update_begin(void);
 extern void cpu_maps_update_done(void);
+extern int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu);
 
 #else	/* CONFIG_SMP */
 #define cpuhp_tasks_frozen	0
diff --git a/kernel/cpu.c b/kernel/cpu.c
index e1967e9eddc2..219f9033f438 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1197,6 +1197,20 @@ int cpu_up(unsigned int cpu)
 }
 EXPORT_SYMBOL_GPL(cpu_up);
 
+int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
+{
+	int ret;
+
+	if (!cpu_online(sleep_cpu)) {
+		pr_info("Hibernated on a CPU that is offline! Bringing CPU up.\n");
+		ret = cpu_up(sleep_cpu);
+		if (ret) {
+			pr_err("Failed to bring hibernate-CPU up!\n");
+			return ret;
+		}
+	}
+}
+
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 02/12] x86: Replace cpu_up/down with devcie_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
  2019-10-30 15:38 ` [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu) Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-10-30 15:38 ` [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline Qais Yousef
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86,
	linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/topology.c | 4 ++--
 arch/x86/mm/mmio-mod.c     | 8 ++++++--
 arch/x86/xen/smp.c         | 4 +++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index be5bc2e47c71..3b253088615e 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -69,7 +69,7 @@ int _debug_hotplug_cpu(int cpu, int action)
 
 	switch (action) {
 	case 0:
-		ret = cpu_down(cpu);
+		ret = device_offline(get_cpu_device(cpu));
 		if (!ret) {
 			pr_info("DEBUG_HOTPLUG_CPU0: CPU %u is now offline\n", cpu);
 			dev->offline = true;
@@ -78,7 +78,7 @@ int _debug_hotplug_cpu(int cpu, int action)
 			pr_debug("Can't offline CPU%d.\n", cpu);
 		break;
 	case 1:
-		ret = cpu_up(cpu);
+		ret = device_online(get_cpu_device(cpu));
 		if (!ret) {
 			dev->offline = false;
 			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index b8ef8557d4b3..7ec7d05335ce 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -385,13 +385,15 @@ static void enter_uniprocessor(void)
 		pr_notice("Disabling non-boot CPUs...\n");
 	put_online_cpus();
 
+	lock_device_hotplug();
 	for_each_cpu(cpu, downed_cpus) {
-		err = cpu_down(cpu);
+		err = device_offline(get_cpu_device(cpu));
 		if (!err)
 			pr_info("CPU%d is down.\n", cpu);
 		else
 			pr_err("Error taking CPU%d down: %d\n", cpu, err);
 	}
+	unlock_device_hotplug();
 out:
 	if (num_online_cpus() > 1)
 		pr_warning("multiple CPUs still online, may miss events.\n");
@@ -405,13 +407,15 @@ static void leave_uniprocessor(void)
 	if (downed_cpus == NULL || cpumask_weight(downed_cpus) == 0)
 		return;
 	pr_notice("Re-enabling CPUs...\n");
+	lock_device_hotplug();
 	for_each_cpu(cpu, downed_cpus) {
-		err = cpu_up(cpu);
+		err = device_online(get_cpu_device(cpu));
 		if (!err)
 			pr_info("enabled CPU%d.\n", cpu);
 		else
 			pr_err("cannot re-enable CPU%d: %d\n", cpu, err);
 	}
+	unlock_device_hotplug();
 }
 
 #else /* !CONFIG_HOTPLUG_CPU */
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 7a43b2ae19f1..aaa31100a31e 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -128,11 +128,12 @@ void __init xen_smp_cpus_done(unsigned int max_cpus)
 	if (xen_have_vcpu_info_placement)
 		return;
 
+	lock_device_hotplug();
 	for_each_online_cpu(cpu) {
 		if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS)
 			continue;
 
-		rc = cpu_down(cpu);
+		rc = device_offline(get_cpu_device(cpu));
 
 		if (rc == 0) {
 			/*
@@ -145,6 +146,7 @@ void __init xen_smp_cpus_done(unsigned int max_cpus)
 				__func__, cpu, rc);
 		}
 	}
+	unlock_device_hotplug();
 	WARN(count, "%s: brought %d CPUs offline\n", __func__, count);
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
  2019-10-30 15:38 ` [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu) Qais Yousef
  2019-10-30 15:38 ` [PATCH 02/12] x86: Replace cpu_up/down with devcie_online/offline Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-19  1:19   ` Michael Ellerman
  2019-10-30 15:38 ` [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus Qais Yousef
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Enrico Weigelt, Ram Pai, Nicholas Piggin,
	Thiago Jung Bauermann, Christophe Leroy, linuxppc-dev,
	linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Enrico Weigelt <info@metux.net>
CC: Ram Pai <linuxram@us.ibm.com>
CC: Nicholas Piggin <npiggin@gmail.com>
CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
CC: Christophe Leroy <christophe.leroy@c-s.fr>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-kernel@vger.kernel.org
---
 arch/powerpc/kernel/machine_kexec_64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 04a7cba58eff..ebf8cc7acc4d 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -208,13 +208,15 @@ static void wake_offline_cpus(void)
 {
 	int cpu = 0;
 
+	lock_device_hotplug();
 	for_each_present_cpu(cpu) {
 		if (!cpu_online(cpu)) {
 			printk(KERN_INFO "kexec: Waking offline cpu %d.\n",
 			       cpu);
-			WARN_ON(cpu_up(cpu));
+			WARN_ON(device_online(get_cpu_device(cpu)));
 		}
 	}
+	unlock_device_hotplug();
 }
 
 static void kexec_prepare_cpus(void)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (2 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-02  4:59   ` kbuild test robot
  2019-11-19 22:21   ` Thomas Gleixner
  2019-10-30 15:38 ` [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline Qais Yousef
                   ` (7 subsequent siblings)
  11 siblings, 2 replies; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

Use freeze_secondary_cpus() instead of open coding using cpu_down()
directly.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/ia64/kernel/process.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 968b5f33e725..70b433eafa5c 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -647,12 +647,8 @@ cpu_halt (void)
 void machine_shutdown(void)
 {
 #ifdef CONFIG_HOTPLUG_CPU
-	int cpu;
-
-	for_each_online_cpu(cpu) {
-		if (cpu != smp_processor_id())
-			cpu_down(cpu);
-	}
+	/* TODO: Can we use disable_nonboot_cpus()? */
+	freeze_secondary_cpus(smp_processor_id());
 #endif
 #ifdef CONFIG_KEXEC
 	kexec_disable_iosapic();
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (3 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-10-30 19:33   ` David Miller
  2019-10-30 15:38 ` [PATCH 06/12] parisc: " Qais Yousef
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, David S. Miller, Bjorn Helgaas, Rafael J. Wysocki,
	Sakari Ailus, sparclinux, linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Sakari Ailus <sakari.ailus@linux.intel.com>
CC: sparclinux@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/sparc/kernel/ds.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/ds.c b/arch/sparc/kernel/ds.c
index bbf59b3b4af8..39350e5dbaf1 100644
--- a/arch/sparc/kernel/ds.c
+++ b/arch/sparc/kernel/ds.c
@@ -550,12 +550,13 @@ static int dr_cpu_configure(struct ds_info *dp, struct ds_cap_state *cp,
 	mdesc_populate_present_mask(mask);
 	mdesc_fill_in_cpu_data(mask);
 
+	lock_device_hotplug();
 	for_each_cpu(cpu, mask) {
 		int err;
 
 		printk(KERN_INFO "ds-%llu: Starting cpu %d...\n",
 		       dp->id, cpu);
-		err = cpu_up(cpu);
+		err = device_online(get_cpu_device(cpu));
 		if (err) {
 			__u32 res = DR_CPU_RES_FAILURE;
 			__u32 stat = DR_CPU_STAT_UNCONFIGURED;
@@ -574,6 +575,7 @@ static int dr_cpu_configure(struct ds_info *dp, struct ds_cap_state *cp,
 			dr_cpu_mark(resp, cpu, ncpus, res, stat);
 		}
 	}
+	unlock_device_hotplug();
 
 	spin_lock_irqsave(&ds_lock, flags);
 	__ds_send(dp->lp, resp, resp_len);
@@ -606,17 +608,19 @@ static int dr_cpu_unconfigure(struct ds_info *dp,
 			     resp_len, ncpus, mask,
 			     DR_CPU_STAT_UNCONFIGURED);
 
+	lock_device_hotplug();
 	for_each_cpu(cpu, mask) {
 		int err;
 
 		printk(KERN_INFO "ds-%llu: Shutting down cpu %d...\n",
 		       dp->id, cpu);
-		err = cpu_down(cpu);
+		err = device_offline(get_cpu_device(cpu));
 		if (err)
 			dr_cpu_mark(resp, cpu, ncpus,
 				    DR_CPU_RES_FAILURE,
 				    DR_CPU_STAT_CONFIGURED);
 	}
+	unlock_device_hotplug();
 
 	spin_lock_irqsave(&ds_lock, flags);
 	__ds_send(dp->lp, resp, resp_len);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 06/12] parisc: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (4 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-20 11:09   ` Qais Yousef
  2019-10-30 15:38 ` [PATCH 07/12] driver: base: cpu: export device_online/offline Qais Yousef
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, James E.J. Bottomley, Helge Deller, Richard Fontana,
	Armijn Hemel, linux-parisc, linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
CC: Helge Deller <deller@gmx.de>
CC: Richard Fontana <rfontana@redhat.com>
CC: Armijn Hemel <armijn@tjaldur.nl>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux-parisc@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---

Couldn't compile test this one.

I'm not confident that this is a correct patch to be honest. This __init
indicates we're booting the secondary cpus and that might be too early in the
process to use the core API..?


 arch/parisc/kernel/processor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
index 13f771f74ee3..4dde5fe78f0c 100644
--- a/arch/parisc/kernel/processor.c
+++ b/arch/parisc/kernel/processor.c
@@ -212,7 +212,9 @@ static int __init processor_probe(struct parisc_device *dev)
 #ifdef CONFIG_SMP
 	if (cpuid) {
 		set_cpu_present(cpuid, true);
-		cpu_up(cpuid);
+		lock_device_hotplug();
+		device_online(get_cpu_device(cpuid));
+		unlock_device_hotplug();
 	}
 #endif
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07/12] driver: base: cpu: export device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (5 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 06/12] parisc: " Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-02 17:17   ` Greg Kroah-Hartman
  2019-10-30 15:38 ` [PATCH 08/12] driver: xen: Replace cpu_up/down with device_online/offline Qais Yousef
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Rafael J. Wysocki, linux-kernel

And the {lock,unlock}_device_hotplug so that they can be used from
modules.

This is in preparation to replace cpu_up/down with
device_online/offline; which kernel/torture.c will require to be
exported to be built as a module.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: "Rafael J. Wysocki" <rafael@kernel.org>
CC: linux-kernel@vger.kernel.org
---
 drivers/base/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2db62d98e395..3431cd0c9eac 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -882,11 +882,13 @@ void lock_device_hotplug(void)
 {
 	mutex_lock(&device_hotplug_lock);
 }
+EXPORT_SYMBOL_GPL(lock_device_hotplug);
 
 void unlock_device_hotplug(void)
 {
 	mutex_unlock(&device_hotplug_lock);
 }
+EXPORT_SYMBOL_GPL(unlock_device_hotplug);
 
 int lock_device_hotplug_sysfs(void)
 {
@@ -2678,6 +2680,7 @@ int device_offline(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(device_offline);
 
 /**
  * device_online - Put the device back online after successful device_offline().
@@ -2709,6 +2712,7 @@ int device_online(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(device_online);
 
 struct root_device {
 	struct device dev;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/12] driver: xen: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (6 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 07/12] driver: base: cpu: export device_online/offline Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-10-30 15:38 ` [PATCH 09/12] firmware: psci: " Qais Yousef
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Boris Ostrovsky, Juergen Gross, Stefano Stabellini,
	xen-devel, linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Juergen Gross <jgross@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: xen-devel@lists.xenproject.org
CC: linux-kernel@vger.kernel.org
---
 drivers/xen/cpu_hotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index f192b6f42da9..ec975decb5de 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -94,7 +94,7 @@ static int setup_cpu_watcher(struct notifier_block *notifier,
 
 	for_each_possible_cpu(cpu) {
 		if (vcpu_online(cpu) == 0) {
-			(void)cpu_down(cpu);
+			device_offline(get_cpu_device(cpu));
 			set_cpu_present(cpu, false);
 		}
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 09/12] firmware: psci: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (7 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 08/12] driver: xen: Replace cpu_up/down with device_online/offline Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-10-30 15:38 ` [PATCH 10/12] torture: " Qais Yousef
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Mark Rutland, Lorenzo Pieralisi, linux-arm-kernel,
	linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---
 drivers/firmware/psci/psci_checker.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/psci/psci_checker.c b/drivers/firmware/psci/psci_checker.c
index 6a445397771c..9e4b1bade659 100644
--- a/drivers/firmware/psci/psci_checker.c
+++ b/drivers/firmware/psci/psci_checker.c
@@ -83,8 +83,9 @@ static unsigned int down_and_up_cpus(const struct cpumask *cpus,
 	cpumask_clear(offlined_cpus);
 
 	/* Try to power down all CPUs in the mask. */
+	lock_device_hotplug();
 	for_each_cpu(cpu, cpus) {
-		int ret = cpu_down(cpu);
+		int ret = device_offline(get_cpu_device(cpu));
 
 		/*
 		 * cpu_down() checks the number of online CPUs before the TOS
@@ -116,7 +117,7 @@ static unsigned int down_and_up_cpus(const struct cpumask *cpus,
 
 	/* Try to power up all the CPUs that have been offlined. */
 	for_each_cpu(cpu, offlined_cpus) {
-		int ret = cpu_up(cpu);
+		int ret = device_online(get_cpu_device(cpu));
 
 		if (ret != 0) {
 			pr_err("Error occurred (%d) while trying "
@@ -126,6 +127,7 @@ static unsigned int down_and_up_cpus(const struct cpumask *cpus,
 			cpumask_clear_cpu(cpu, offlined_cpus);
 		}
 	}
+	unlock_device_hotplug();
 
 	/*
 	 * Something went bad at some point and some CPUs could not be turned
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 10/12] torture: Replace cpu_up/down with device_online/offline
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (8 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 09/12] firmware: psci: " Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-10-30 15:38 ` [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online Qais Yousef
  2019-10-30 15:38 ` [PATCH 12/12] cpu: Hide cpu_up/down Qais Yousef
  11 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
	linux-kernel

The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.

See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.

This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Davidlohr Bueso <dave@stgolabs.net>
CC: "Paul E. McKenney" <paulmck@kernel.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: linux-kernel@vger.kernel.org
---
 kernel/torture.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/torture.c b/kernel/torture.c
index 7c13f5558b71..12115024feb2 100644
--- a/kernel/torture.c
+++ b/kernel/torture.c
@@ -97,7 +97,9 @@ bool torture_offline(int cpu, long *n_offl_attempts, long *n_offl_successes,
 			 torture_type, cpu);
 	starttime = jiffies;
 	(*n_offl_attempts)++;
-	ret = cpu_down(cpu);
+	lock_device_hotplug();
+	ret = device_offline(get_cpu_device(cpu));
+	unlock_device_hotplug();
 	if (ret) {
 		if (verbose)
 			pr_alert("%s" TORTURE_FLAG
@@ -148,7 +150,9 @@ bool torture_online(int cpu, long *n_onl_attempts, long *n_onl_successes,
 			 torture_type, cpu);
 	starttime = jiffies;
 	(*n_onl_attempts)++;
-	ret = cpu_up(cpu);
+	lock_device_hotplug();
+	ret = device_online(get_cpu_device(cpu));
+	unlock_device_hotplug();
 	if (ret) {
 		if (verbose)
 			pr_alert("%s" TORTURE_FLAG
@@ -192,17 +196,20 @@ torture_onoff(void *arg)
 	for_each_online_cpu(cpu)
 		maxcpu = cpu;
 	WARN_ON(maxcpu < 0);
-	if (!IS_MODULE(CONFIG_TORTURE_TEST))
+	if (!IS_MODULE(CONFIG_TORTURE_TEST)) {
+		lock_device_hotplug();
 		for_each_possible_cpu(cpu) {
 			if (cpu_online(cpu))
 				continue;
-			ret = cpu_up(cpu);
+			ret = device_online(get_cpu_device(cpu));
 			if (ret && verbose) {
 				pr_alert("%s" TORTURE_FLAG
 					 "%s: Initial online %d: errno %d\n",
 					 __func__, torture_type, cpu, ret);
 			}
 		}
+		unlock_device_hotplug();
+	}
 
 	if (maxcpu == 0) {
 		VERBOSE_TOROUT_STRING("Only one CPU, so CPU-hotplug testing is disabled");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (9 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 10/12] torture: " Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-19 22:42   ` Thomas Gleixner
  2019-10-30 15:38 ` [PATCH 12/12] cpu: Hide cpu_up/down Qais Yousef
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Josh Poimboeuf, Peter Zijlstra (Intel),
	Jiri Kosina, Nicholas Piggin, Daniel Lezcano, Ingo Molnar,
	Eiichi Tsukata, Zhenzhong Duan, Nadav Amit, Rafael J. Wysocki,
	linux-kernel

This is the last direct user of cpu_up() before we can hide it now as
internal implementation detail of the cpu subsystem.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Josh Poimboeuf <jpoimboe@redhat.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: Jiri Kosina <jkosina@suse.cz>
CC: Nicholas Piggin <npiggin@gmail.com>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Eiichi Tsukata <devel@etsukata.com>
CC: Zhenzhong Duan <zhenzhong.duan@oracle.com>
CC: Nadav Amit <namit@vmware.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
CC: linux-kernel@vger.kernel.org
---
 include/linux/cpu.h |  1 +
 kernel/cpu.c        | 13 +++++++++++++
 kernel/smp.c        |  9 +--------
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 3b1fbe192989..b1c7b788b8e9 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -88,6 +88,7 @@ void notify_cpu_starting(unsigned int cpu);
 extern void cpu_maps_update_begin(void);
 extern void cpu_maps_update_done(void);
 extern int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu);
+extern void smp_bringup_nonboot_cpus(unsigned int setup_max_cpus);
 
 #else	/* CONFIG_SMP */
 #define cpuhp_tasks_frozen	0
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 219f9033f438..e16695b841de 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1211,6 +1211,19 @@ int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
 	}
 }
 
+void smp_bringup_nonboot_cpus(unsigned int setup_max_cpus)
+{
+	unsigned int cpu;
+
+	/* FIXME: This should be done in userspace --RR */
+	for_each_present_cpu(cpu) {
+		if (num_online_cpus() >= setup_max_cpus)
+			break;
+		if (!cpu_online(cpu))
+			cpu_up(cpu);
+	}
+}
+
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
 
diff --git a/kernel/smp.c b/kernel/smp.c
index 7dbcb402c2fc..74134272b5aa 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -578,20 +578,13 @@ void __init setup_nr_cpu_ids(void)
 void __init smp_init(void)
 {
 	int num_nodes, num_cpus;
-	unsigned int cpu;
 
 	idle_threads_init();
 	cpuhp_threads_init();
 
 	pr_info("Bringing up secondary CPUs ...\n");
 
-	/* FIXME: This should be done in userspace --RR */
-	for_each_present_cpu(cpu) {
-		if (num_online_cpus() >= setup_max_cpus)
-			break;
-		if (!cpu_online(cpu))
-			cpu_up(cpu);
-	}
+	smp_bringup_nonboot_cpus(setup_max_cpus);
 
 	num_nodes = num_online_nodes();
 	num_cpus  = num_online_cpus();
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 12/12] cpu: Hide cpu_up/down
       [not found] <20191030153837.18107-1-qais.yousef@arm.com>
                   ` (10 preceding siblings ...)
  2019-10-30 15:38 ` [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online Qais Yousef
@ 2019-10-30 15:38 ` Qais Yousef
  2019-11-19 22:25   ` Thomas Gleixner
  11 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Rafael J. Wysocki, Josh Poimboeuf, Nicholas Piggin,
	Peter Zijlstra (Intel),
	Jiri Kosina, Daniel Lezcano, Eiichi Tsukata, Zhenzhong Duan,
	Ingo Molnar, Pavankumar Kondeti, linux-kernel

Provide a special exported function for the device core to bring a cpu
up/down and hide the real cpu_up/down as they are treated as private
functions. cpu_up/down are lower level API and users outside the cpu
subsystem should use device_online/offline which will take care of extra
housekeeping work like keeping sysfs in sync.

Signed-off-by: Qais Yousef <qais.yousef@arm.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: "Rafael J. Wysocki" <rafael@kernel.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Josh Poimboeuf <jpoimboe@redhat.com>
CC: Nicholas Piggin <npiggin@gmail.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: Jiri Kosina <jkosina@suse.cz>
CC: Daniel Lezcano <daniel.lezcano@linaro.org>
CC: Eiichi Tsukata <devel@etsukata.com>
CC: Zhenzhong Duan <zhenzhong.duan@oracle.com>
CC: Ingo Molnar <mingo@kernel.org>
CC: Pavankumar Kondeti <pkondeti@codeaurora.org>
CC: linux-kernel@vger.kernel.org
---
 drivers/base/cpu.c  |  4 ++--
 include/linux/cpu.h |  4 ++--
 kernel/cpu.c        | 26 ++++++++++++++++++++++----
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index cc37511de866..96c69c5fbfff 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -55,7 +55,7 @@ static int cpu_subsys_online(struct device *dev)
 	if (from_nid == NUMA_NO_NODE)
 		return -ENODEV;
 
-	ret = cpu_up(cpuid);
+	ret = cpu_subsys_up(dev);
 	/*
 	 * When hot adding memory to memoryless node and enabling a cpu
 	 * on the node, node number of the cpu may internally change.
@@ -69,7 +69,7 @@ static int cpu_subsys_online(struct device *dev)
 
 static int cpu_subsys_offline(struct device *dev)
 {
-	return cpu_down(dev->id);
+	return cpu_subsys_down(dev);
 }
 
 void unregister_cpu(struct cpu *cpu)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index b1c7b788b8e9..6822e676f420 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -83,7 +83,7 @@ extern ssize_t arch_cpu_release(const char *, size_t);
 
 #ifdef CONFIG_SMP
 extern bool cpuhp_tasks_frozen;
-int cpu_up(unsigned int cpu);
+int cpu_subsys_up(struct device *dev);
 void notify_cpu_starting(unsigned int cpu);
 extern void cpu_maps_update_begin(void);
 extern void cpu_maps_update_done(void);
@@ -114,7 +114,7 @@ extern void lockdep_assert_cpus_held(void);
 extern void cpu_hotplug_disable(void);
 extern void cpu_hotplug_enable(void);
 void clear_tasks_mm_cpumask(int cpu);
-int cpu_down(unsigned int cpu);
+int cpu_subsys_down(struct device *dev);
 
 #else /* CONFIG_HOTPLUG_CPU */
 
diff --git a/kernel/cpu.c b/kernel/cpu.c
index e16695b841de..087c10dbfb99 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1045,11 +1045,20 @@ static int do_cpu_down(unsigned int cpu, enum cpuhp_state target)
 	return err;
 }
 
-int cpu_down(unsigned int cpu)
+static int cpu_down(unsigned int cpu)
 {
 	return do_cpu_down(cpu, CPUHP_OFFLINE);
 }
-EXPORT_SYMBOL(cpu_down);
+
+/*
+ * This function is meant to be used by device core cpu subsystem.
+ *
+ * Other subsystems should use device_offline(get_cpu_device(cpu)) instead.
+ */
+int cpu_subsys_down(struct device *dev)
+{
+	return cpu_down(dev->id);
+}
 
 #else
 #define takedown_cpu		NULL
@@ -1191,11 +1200,20 @@ static int do_cpu_up(unsigned int cpu, enum cpuhp_state target)
 	return err;
 }
 
-int cpu_up(unsigned int cpu)
+static int cpu_up(unsigned int cpu)
 {
 	return do_cpu_up(cpu, CPUHP_ONLINE);
 }
-EXPORT_SYMBOL_GPL(cpu_up);
+
+/*
+ * This function is meant to be used by device core cpu subsystem.
+ *
+ * Other subsystems should use device_online(get_cpu_device(cpu)) instead.
+ */
+int cpu_subsys_up(struct device *dev)
+{
+	return cpu_up(dev->id);
+}
 
 int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline
  2019-10-30 15:38 ` [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline Qais Yousef
@ 2019-10-30 19:33   ` David Miller
  0 siblings, 0 replies; 33+ messages in thread
From: David Miller @ 2019-10-30 19:33 UTC (permalink / raw)
  To: qais.yousef
  Cc: tglx, gregkh, bhelgaas, rafael.j.wysocki, sakari.ailus,
	sparclinux, linux-kernel

From: Qais Yousef <qais.yousef@arm.com>
Date: Wed, 30 Oct 2019 15:38:30 +0000

> The core device API performs extra housekeeping bits that are missing
> from directly calling cpu_up/down.
> 
> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> serialization during LPM") for an example description of what might go
> wrong.
> 
> This also prepares to make cpu_up/down a private interface for anything
> but the cpu subsystem.
> 
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-10-30 15:38 ` [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus Qais Yousef
@ 2019-11-02  4:59   ` kbuild test robot
  2019-11-19 22:21   ` Thomas Gleixner
  1 sibling, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2019-11-02  4:59 UTC (permalink / raw)
  To: Qais Yousef
  Cc: kbuild-all, Thomas Gleixner, Greg Kroah-Hartman, Qais Yousef,
	Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]

Hi Qais,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.4-rc5 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Qais-Yousef/arm64-hibernate-c-create-a-new-function-to-handle-cpu_up-sleep_cpu/20191102-053138
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0dbe6cb8f7e05bc9611602ef45980a6c57b245a3
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/ia64/kernel/process.c: In function 'machine_shutdown':
>> arch/ia64/kernel/process.c:651:2: error: implicit declaration of function 'freeze_secondary_cpus'; did you mean 'suspend_enable_secondary_cpus'? [-Werror=implicit-function-declaration]
     freeze_secondary_cpus(smp_processor_id());
     ^~~~~~~~~~~~~~~~~~~~~
     suspend_enable_secondary_cpus
   cc1: some warnings being treated as errors

vim +651 arch/ia64/kernel/process.c

   646	
   647	void machine_shutdown(void)
   648	{
   649	#ifdef CONFIG_HOTPLUG_CPU
   650		/* TODO: Can we use disable_nonboot_cpus()? */
 > 651		freeze_secondary_cpus(smp_processor_id());
   652	#endif
   653	#ifdef CONFIG_KEXEC
   654		kexec_disable_iosapic();
   655	#endif
   656	}
   657	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54922 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 07/12] driver: base: cpu: export device_online/offline
  2019-10-30 15:38 ` [PATCH 07/12] driver: base: cpu: export device_online/offline Qais Yousef
@ 2019-11-02 17:17   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2019-11-02 17:17 UTC (permalink / raw)
  To: Qais Yousef; +Cc: Thomas Gleixner, Rafael J. Wysocki, linux-kernel

On Wed, Oct 30, 2019 at 03:38:32PM +0000, Qais Yousef wrote:
> And the {lock,unlock}_device_hotplug so that they can be used from
> modules.
> 
> This is in preparation to replace cpu_up/down with
> device_online/offline; which kernel/torture.c will require to be
> exported to be built as a module.
> 
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: "Rafael J. Wysocki" <rafael@kernel.org>
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/base/core.c | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline
  2019-10-30 15:38 ` [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline Qais Yousef
@ 2019-11-19  1:19   ` Michael Ellerman
  2019-11-19  9:47     ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Michael Ellerman @ 2019-11-19  1:19 UTC (permalink / raw)
  To: Qais Yousef, Thomas Gleixner, Greg Kroah-Hartman
  Cc: Qais Yousef, Benjamin Herrenschmidt, Paul Mackerras,
	Enrico Weigelt, Ram Pai, Nicholas Piggin, Thiago Jung Bauermann,
	Christophe Leroy, linuxppc-dev, linux-kernel

Qais Yousef <qais.yousef@arm.com> writes:
> The core device API performs extra housekeeping bits that are missing
> from directly calling cpu_up/down.
>
> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> serialization during LPM") for an example description of what might go
> wrong.
>
> This also prepares to make cpu_up/down a private interface for anything
> but the cpu subsystem.
>
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Paul Mackerras <paulus@samba.org>
> CC: Michael Ellerman <mpe@ellerman.id.au>
> CC: Enrico Weigelt <info@metux.net>
> CC: Ram Pai <linuxram@us.ibm.com>
> CC: Nicholas Piggin <npiggin@gmail.com>
> CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> CC: Christophe Leroy <christophe.leroy@c-s.fr>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/powerpc/kernel/machine_kexec_64.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

My initial though is "what about kdump", but that function is not called
during kdump, so there should be no issue with the extra locking leading
to deadlocks in a crash.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

I assume you haven't actually tested it?

cheers

> diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
> index 04a7cba58eff..ebf8cc7acc4d 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -208,13 +208,15 @@ static void wake_offline_cpus(void)
>  {
>  	int cpu = 0;
>  
> +	lock_device_hotplug();
>  	for_each_present_cpu(cpu) {
>  		if (!cpu_online(cpu)) {
>  			printk(KERN_INFO "kexec: Waking offline cpu %d.\n",
>  			       cpu);
> -			WARN_ON(cpu_up(cpu));
> +			WARN_ON(device_online(get_cpu_device(cpu)));
>  		}
>  	}
> +	unlock_device_hotplug();
>  }
>  
>  static void kexec_prepare_cpus(void)
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline
  2019-11-19  1:19   ` Michael Ellerman
@ 2019-11-19  9:47     ` Qais Yousef
  0 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-11-19  9:47 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Thomas Gleixner, Greg Kroah-Hartman, Benjamin Herrenschmidt,
	Paul Mackerras, Enrico Weigelt, Ram Pai, Nicholas Piggin,
	Thiago Jung Bauermann, Christophe Leroy, linuxppc-dev,
	linux-kernel

On 11/19/19 12:19, Michael Ellerman wrote:
> Qais Yousef <qais.yousef@arm.com> writes:
> > The core device API performs extra housekeeping bits that are missing
> > from directly calling cpu_up/down.
> >
> > See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> > serialization during LPM") for an example description of what might go
> > wrong.
> >
> > This also prepares to make cpu_up/down a private interface for anything
> > but the cpu subsystem.
> >
> > Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: Paul Mackerras <paulus@samba.org>
> > CC: Michael Ellerman <mpe@ellerman.id.au>
> > CC: Enrico Weigelt <info@metux.net>
> > CC: Ram Pai <linuxram@us.ibm.com>
> > CC: Nicholas Piggin <npiggin@gmail.com>
> > CC: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > CC: Christophe Leroy <christophe.leroy@c-s.fr>
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > CC: linuxppc-dev@lists.ozlabs.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >  arch/powerpc/kernel/machine_kexec_64.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> My initial though is "what about kdump", but that function is not called
> during kdump, so there should be no issue with the extra locking leading
> to deadlocks in a crash.
> 
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>

Thanks.

> 
> I assume you haven't actually tested it?

Only compile tested it I'm afraid. Would appreciate if you can give it a spin.
Otherwise I'd be happy to try it out on qemu.

Cheers

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-10-30 15:38 ` [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus Qais Yousef
  2019-11-02  4:59   ` kbuild test robot
@ 2019-11-19 22:21   ` Thomas Gleixner
  2019-11-19 22:32     ` Qais Yousef
  1 sibling, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:21 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On Wed, 30 Oct 2019, Qais Yousef wrote:

> Use freeze_secondary_cpus() instead of open coding using cpu_down()
> directly.
> 
> This also prepares to make cpu_up/down a private interface for anything
> but the cpu subsystem.
> 
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> CC: Tony Luck <tony.luck@intel.com>
> CC: Fenghua Yu <fenghua.yu@intel.com>
> CC: linux-ia64@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/ia64/kernel/process.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
> index 968b5f33e725..70b433eafa5c 100644
> --- a/arch/ia64/kernel/process.c
> +++ b/arch/ia64/kernel/process.c
> @@ -647,12 +647,8 @@ cpu_halt (void)
>  void machine_shutdown(void)
>  {
>  #ifdef CONFIG_HOTPLUG_CPU
> -	int cpu;
> -
> -	for_each_online_cpu(cpu) {
> -		if (cpu != smp_processor_id())
> -			cpu_down(cpu);
> -	}
> +	/* TODO: Can we use disable_nonboot_cpus()? */
> +	freeze_secondary_cpus(smp_processor_id());

freeze_secondary_cpus() is only available for CONFIG_PM_SLEEP_SMP=y and
disable_nonboot_cpus() is a NOOP for CONFIG_PM_SLEEP_SMP=n :)

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 12/12] cpu: Hide cpu_up/down
  2019-10-30 15:38 ` [PATCH 12/12] cpu: Hide cpu_up/down Qais Yousef
@ 2019-11-19 22:25   ` Thomas Gleixner
  2019-11-19 22:36     ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:25 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Josh Poimboeuf,
	Nicholas Piggin, Peter Zijlstra (Intel),
	Jiri Kosina, Daniel Lezcano, Eiichi Tsukata, Zhenzhong Duan,
	Ingo Molnar, Pavankumar Kondeti, linux-kernel

On Wed, 30 Oct 2019, Qais Yousef wrote:
> -int cpu_down(unsigned int cpu)
> +static int cpu_down(unsigned int cpu)
>  {
>  	return do_cpu_down(cpu, CPUHP_OFFLINE);
>  }
> -EXPORT_SYMBOL(cpu_down);

The exports should be gone already at this point, right?

> +/*
> + * This function is meant to be used by device core cpu subsystem.
> + *
> + * Other subsystems should use device_offline(get_cpu_device(cpu)) instead.
> + */

Can you please use proper kernel-doc function documentation?

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu)
  2019-10-30 15:38 ` [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu) Qais Yousef
@ 2019-11-19 22:31   ` Thomas Gleixner
  2019-11-19 22:51     ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:31 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Steve Capper,
	Richard Fontana, James Morse, Mark Rutland, Josh Poimboeuf,
	Ingo Molnar, Peter Zijlstra (Intel),
	Nicholas Piggin, Daniel Lezcano, Jiri Kosina, Pavankumar Kondeti,
	Zhenzhong Duan, linux-arm-kernel, linux-kernel

On Wed, 30 Oct 2019, Qais Yousef wrote:
>  
> +int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)

That function name is horrible. Aside of that I really have to ask how you
end up hibernating on an offline CPU?

> +{
> +	int ret;
> +
> +	if (!cpu_online(sleep_cpu)) {
> +		pr_info("Hibernated on a CPU that is offline! Bringing CPU up.\n");
> +		ret = cpu_up(sleep_cpu);
> +		if (ret) {
> +			pr_err("Failed to bring hibernate-CPU up!\n");
> +			return ret;
> +		}
> +	}
> +}

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-11-19 22:21   ` Thomas Gleixner
@ 2019-11-19 22:32     ` Qais Yousef
  2019-11-19 22:59       ` Thomas Gleixner
  0 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-11-19 22:32 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On 11/19/19 23:21, Thomas Gleixner wrote:
> On Wed, 30 Oct 2019, Qais Yousef wrote:
> 
> > Use freeze_secondary_cpus() instead of open coding using cpu_down()
> > directly.
> > 
> > This also prepares to make cpu_up/down a private interface for anything
> > but the cpu subsystem.
> > 
> > Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> > CC: Tony Luck <tony.luck@intel.com>
> > CC: Fenghua Yu <fenghua.yu@intel.com>
> > CC: linux-ia64@vger.kernel.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >  arch/ia64/kernel/process.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
> > index 968b5f33e725..70b433eafa5c 100644
> > --- a/arch/ia64/kernel/process.c
> > +++ b/arch/ia64/kernel/process.c
> > @@ -647,12 +647,8 @@ cpu_halt (void)
> >  void machine_shutdown(void)
> >  {
> >  #ifdef CONFIG_HOTPLUG_CPU
> > -	int cpu;
> > -
> > -	for_each_online_cpu(cpu) {
> > -		if (cpu != smp_processor_id())
> > -			cpu_down(cpu);
> > -	}
> > +	/* TODO: Can we use disable_nonboot_cpus()? */
> > +	freeze_secondary_cpus(smp_processor_id());
> 
> freeze_secondary_cpus() is only available for CONFIG_PM_SLEEP_SMP=y and
> disable_nonboot_cpus() is a NOOP for CONFIG_PM_SLEEP_SMP=n :)

I thought I replied to this :-(

My plan was to simply make freeze_secondary_cpus() available and protected by
CONFIG_SMP only instead.

Good plan?

I'll probably do it as a separate patch before this one.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 12/12] cpu: Hide cpu_up/down
  2019-11-19 22:25   ` Thomas Gleixner
@ 2019-11-19 22:36     ` Qais Yousef
  0 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-11-19 22:36 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Josh Poimboeuf,
	Nicholas Piggin, Peter Zijlstra (Intel),
	Jiri Kosina, Daniel Lezcano, Eiichi Tsukata, Zhenzhong Duan,
	Ingo Molnar, Pavankumar Kondeti, linux-kernel

On 11/19/19 23:25, Thomas Gleixner wrote:
> On Wed, 30 Oct 2019, Qais Yousef wrote:
> > -int cpu_down(unsigned int cpu)
> > +static int cpu_down(unsigned int cpu)
> >  {
> >  	return do_cpu_down(cpu, CPUHP_OFFLINE);
> >  }
> > -EXPORT_SYMBOL(cpu_down);
> 
> The exports should be gone already at this point, right?

Yes. The only in-kernel user was the torture test.

> > +/*
> > + * This function is meant to be used by device core cpu subsystem.
> > + *
> > + * Other subsystems should use device_offline(get_cpu_device(cpu)) instead.
> > + */
> 
> Can you please use proper kernel-doc function documentation?

Will do.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online
  2019-10-30 15:38 ` [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online Qais Yousef
@ 2019-11-19 22:42   ` Thomas Gleixner
  0 siblings, 0 replies; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:42 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Josh Poimboeuf, Peter Zijlstra (Intel),
	Jiri Kosina, Nicholas Piggin, Daniel Lezcano, Ingo Molnar,
	Eiichi Tsukata, Zhenzhong Duan, Nadav Amit, Rafael J. Wysocki,
	linux-kernel

On Wed, 30 Oct 2019, Qais Yousef wrote:
> +void smp_bringup_nonboot_cpus(unsigned int setup_max_cpus)
> +{
> +	unsigned int cpu;
> +
> +	/* FIXME: This should be done in userspace --RR */

This fixme is stale and should just go away.

> +	for_each_present_cpu(cpu) {
> +		if (num_online_cpus() >= setup_max_cpus)
> +			break;
> +		if (!cpu_online(cpu))
> +			cpu_up(cpu);
> +	}
> +}

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu)
  2019-11-19 22:31   ` Thomas Gleixner
@ 2019-11-19 22:51     ` Qais Yousef
  2019-11-19 23:01       ` Thomas Gleixner
  0 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-11-19 22:51 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Steve Capper,
	Richard Fontana, James Morse, Mark Rutland, Josh Poimboeuf,
	Ingo Molnar, Peter Zijlstra (Intel),
	Nicholas Piggin, Daniel Lezcano, Jiri Kosina, Pavankumar Kondeti,
	Zhenzhong Duan, linux-arm-kernel, linux-kernel

On 11/19/19 23:31, Thomas Gleixner wrote:
> On Wed, 30 Oct 2019, Qais Yousef wrote:
> >  
> > +int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
> 
> That function name is horrible. Aside of that I really have to ask how you
> end up hibernating on an offline CPU?

James Morse can probably explain better.

But AFAIU we could sleep on any CPU, but on the next cold boot that CPU could
become offline as a side effect of using maxcpus= for example.

How about bringup_hibernate_cpu() as a name? I could add the above as an
explanation of why we need this call too.

It does seem to me that this is a generic problem that we might be able to
handle generically, but I'm not sure how.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-11-19 22:32     ` Qais Yousef
@ 2019-11-19 22:59       ` Thomas Gleixner
  2019-11-19 23:19         ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:59 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On Tue, 19 Nov 2019, Qais Yousef wrote:
> On 11/19/19 23:21, Thomas Gleixner wrote:
> > On Wed, 30 Oct 2019, Qais Yousef wrote:
> > >  void machine_shutdown(void)
> > >  {
> > >  #ifdef CONFIG_HOTPLUG_CPU
> > > -	int cpu;
> > > -
> > > -	for_each_online_cpu(cpu) {
> > > -		if (cpu != smp_processor_id())
> > > -			cpu_down(cpu);
> > > -	}
> > > +	/* TODO: Can we use disable_nonboot_cpus()? */
> > > +	freeze_secondary_cpus(smp_processor_id());
> > 
> > freeze_secondary_cpus() is only available for CONFIG_PM_SLEEP_SMP=y and
> > disable_nonboot_cpus() is a NOOP for CONFIG_PM_SLEEP_SMP=n :)
> 
> I thought I replied to this :-(
> 
> My plan was to simply make freeze_secondary_cpus() available and protected by
> CONFIG_SMP only instead.
> 
> Good plan?

No. freeze_secondary_cpus() is really for hibernation. Look at the exit
conditions there.

So you really want a seperate function which depends on CONFIG_HOTPLUG_CPU
and provides an inline stub for the CONFIG_HOTPLUG_CPU=n case.

But I have a hard time to see how that stuff works at all on
ia64:

  cpu_down() might sleep, i.e. it must be called from preemptible
  context. smp_processor_id() is invalid from preemtible context.

As this is obviously broken and ia64 is in keep compile mode only, it
should just go away.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu)
  2019-11-19 22:51     ` Qais Yousef
@ 2019-11-19 23:01       ` Thomas Gleixner
  2019-11-19 23:21         ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-19 23:01 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Steve Capper,
	Richard Fontana, James Morse, Mark Rutland, Josh Poimboeuf,
	Ingo Molnar, Peter Zijlstra (Intel),
	Nicholas Piggin, Daniel Lezcano, Jiri Kosina, Pavankumar Kondeti,
	Zhenzhong Duan, linux-arm-kernel, linux-kernel

On Tue, 19 Nov 2019, Qais Yousef wrote:
> On 11/19/19 23:31, Thomas Gleixner wrote:
> > On Wed, 30 Oct 2019, Qais Yousef wrote:
> > >  
> > > +int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
> > 
> > That function name is horrible. Aside of that I really have to ask how you
> > end up hibernating on an offline CPU?
> 
> James Morse can probably explain better.
> 
> But AFAIU we could sleep on any CPU, but on the next cold boot that CPU could
> become offline as a side effect of using maxcpus= for example.
> 
> How about bringup_hibernate_cpu() as a name? I could add the above as an
> explanation of why we need this call too.
> 
> It does seem to me that this is a generic problem that we might be able to
> handle generically, but I'm not sure how.

Don't know about other architectures, but x86 does not have that issue as
we force hibernation on CPU0 for historical reasons (Broken BIOSes etc.).

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-11-19 22:59       ` Thomas Gleixner
@ 2019-11-19 23:19         ` Qais Yousef
  2019-11-20  8:46           ` Thomas Gleixner
  0 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-11-19 23:19 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On 11/19/19 23:59, Thomas Gleixner wrote:
> On Tue, 19 Nov 2019, Qais Yousef wrote:
> > On 11/19/19 23:21, Thomas Gleixner wrote:
> > > On Wed, 30 Oct 2019, Qais Yousef wrote:
> > > >  void machine_shutdown(void)
> > > >  {
> > > >  #ifdef CONFIG_HOTPLUG_CPU
> > > > -	int cpu;
> > > > -
> > > > -	for_each_online_cpu(cpu) {
> > > > -		if (cpu != smp_processor_id())
> > > > -			cpu_down(cpu);
> > > > -	}
> > > > +	/* TODO: Can we use disable_nonboot_cpus()? */
> > > > +	freeze_secondary_cpus(smp_processor_id());
> > > 
> > > freeze_secondary_cpus() is only available for CONFIG_PM_SLEEP_SMP=y and
> > > disable_nonboot_cpus() is a NOOP for CONFIG_PM_SLEEP_SMP=n :)
> > 
> > I thought I replied to this :-(
> > 
> > My plan was to simply make freeze_secondary_cpus() available and protected by
> > CONFIG_SMP only instead.
> > 
> > Good plan?
> 
> No. freeze_secondary_cpus() is really for hibernation. Look at the exit
> conditions there.

Hmm do you mean the pm_wakeup_pending() abort?

In arm64 we machine_shutdown() calls disable_nonboot_cpus(), which in turn
a wrapper around freeze_secondary_cpus() with 0 passed as an argument.

IIUC this means arm64 could fail to offline all CPUs on machine_shutdown(),
correct?

> 
> So you really want a seperate function which depends on CONFIG_HOTPLUG_CPU
> and provides an inline stub for the CONFIG_HOTPLUG_CPU=n case.
> 
> But I have a hard time to see how that stuff works at all on
> ia64:
> 
>   cpu_down() might sleep, i.e. it must be called from preemptible
>   context. smp_processor_id() is invalid from preemtible context.
> 
> As this is obviously broken and ia64 is in keep compile mode only, it
> should just go away.

If arm64 is doing the wrong thing, then we need a new function anyway.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu)
  2019-11-19 23:01       ` Thomas Gleixner
@ 2019-11-19 23:21         ` Qais Yousef
  0 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-11-19 23:21 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Steve Capper,
	Richard Fontana, James Morse, Mark Rutland, Josh Poimboeuf,
	Ingo Molnar, Peter Zijlstra (Intel),
	Nicholas Piggin, Daniel Lezcano, Jiri Kosina, Pavankumar Kondeti,
	Zhenzhong Duan, linux-arm-kernel, linux-kernel

On 11/20/19 00:01, Thomas Gleixner wrote:
> On Tue, 19 Nov 2019, Qais Yousef wrote:
> > On 11/19/19 23:31, Thomas Gleixner wrote:
> > > On Wed, 30 Oct 2019, Qais Yousef wrote:
> > > >  
> > > > +int hibernation_bringup_sleep_cpu(unsigned int sleep_cpu)
> > > 
> > > That function name is horrible. Aside of that I really have to ask how you
> > > end up hibernating on an offline CPU?
> > 
> > James Morse can probably explain better.
> > 
> > But AFAIU we could sleep on any CPU, but on the next cold boot that CPU could
> > become offline as a side effect of using maxcpus= for example.
> > 
> > How about bringup_hibernate_cpu() as a name? I could add the above as an
> > explanation of why we need this call too.
> > 
> > It does seem to me that this is a generic problem that we might be able to
> > handle generically, but I'm not sure how.
> 
> Don't know about other architectures, but x86 does not have that issue as
> we force hibernation on CPU0 for historical reasons (Broken BIOSes etc.).

I'll avoid making this series bigger then.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-11-19 23:19         ` Qais Yousef
@ 2019-11-20  8:46           ` Thomas Gleixner
  2019-11-20 10:49             ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Gleixner @ 2019-11-20  8:46 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On Tue, 19 Nov 2019, Qais Yousef wrote:
> On 11/19/19 23:59, Thomas Gleixner wrote:
> > On Tue, 19 Nov 2019, Qais Yousef wrote:
> > > My plan was to simply make freeze_secondary_cpus() available and protected by
> > > CONFIG_SMP only instead.
> > > 
> > > Good plan?
> > 
> > No. freeze_secondary_cpus() is really for hibernation. Look at the exit
> > conditions there.
> 
> Hmm do you mean the pm_wakeup_pending() abort?
> 
> In arm64 we machine_shutdown() calls disable_nonboot_cpus(), which in turn
> a wrapper around freeze_secondary_cpus() with 0 passed as an argument.
> 
> IIUC this means arm64 could fail to offline all CPUs on machine_shutdown(),
> correct?

Looks like.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus
  2019-11-20  8:46           ` Thomas Gleixner
@ 2019-11-20 10:49             ` Qais Yousef
  0 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-11-20 10:49 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Greg Kroah-Hartman, Tony Luck, Fenghua Yu, linux-ia64, linux-kernel

On 11/20/19 09:46, Thomas Gleixner wrote:
> On Tue, 19 Nov 2019, Qais Yousef wrote:
> > On 11/19/19 23:59, Thomas Gleixner wrote:
> > > On Tue, 19 Nov 2019, Qais Yousef wrote:
> > > > My plan was to simply make freeze_secondary_cpus() available and protected by
> > > > CONFIG_SMP only instead.
> > > > 
> > > > Good plan?
> > > 
> > > No. freeze_secondary_cpus() is really for hibernation. Look at the exit
> > > conditions there.
> > 
> > Hmm do you mean the pm_wakeup_pending() abort?
> > 
> > In arm64 we machine_shutdown() calls disable_nonboot_cpus(), which in turn
> > a wrapper around freeze_secondary_cpus() with 0 passed as an argument.
> > 
> > IIUC this means arm64 could fail to offline all CPUs on machine_shutdown(),
> > correct?
> 
> Looks like.

Okay I'll double check and introduce a new function to be called from
machine_down() for arm64 and ia64 if necessary.

Thanks

--
Qais Yousef

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] parisc: Replace cpu_up/down with device_online/offline
  2019-10-30 15:38 ` [PATCH 06/12] parisc: " Qais Yousef
@ 2019-11-20 11:09   ` Qais Yousef
  2019-11-22 19:51     ` Helge Deller
  0 siblings, 1 reply; 33+ messages in thread
From: Qais Yousef @ 2019-11-20 11:09 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller
  Cc: Richard Fontana, Armijn Hemel, linux-parisc, linux-kernel,
	Thomas Gleixner, Greg Kroah-Hartman

On 10/30/19 15:38, Qais Yousef wrote:
> The core device API performs extra housekeeping bits that are missing
> from directly calling cpu_up/down.
> 
> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> serialization during LPM") for an example description of what might go
> wrong.
> 
> This also prepares to make cpu_up/down a private interface for anything
> but the cpu subsystem.
> 
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> CC: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> CC: Helge Deller <deller@gmx.de>
> CC: Richard Fontana <rfontana@redhat.com>
> CC: Armijn Hemel <armijn@tjaldur.nl>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux-parisc@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> 
> Couldn't compile test this one.
> 
> I'm not confident that this is a correct patch to be honest. This __init
> indicates we're booting the secondary cpus and that might be too early in the
> process to use the core API..?

Helge, James

Do you have any comment on this? I have no means to test it and I'd
appreciate if you can spin it through one of your systems.

Thanks

--
Qais Yousef

> 
> 
>  arch/parisc/kernel/processor.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
> index 13f771f74ee3..4dde5fe78f0c 100644
> --- a/arch/parisc/kernel/processor.c
> +++ b/arch/parisc/kernel/processor.c
> @@ -212,7 +212,9 @@ static int __init processor_probe(struct parisc_device *dev)
>  #ifdef CONFIG_SMP
>  	if (cpuid) {
>  		set_cpu_present(cpuid, true);
> -		cpu_up(cpuid);
> +		lock_device_hotplug();
> +		device_online(get_cpu_device(cpuid));
> +		unlock_device_hotplug();
>  	}
>  #endif
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] parisc: Replace cpu_up/down with device_online/offline
  2019-11-20 11:09   ` Qais Yousef
@ 2019-11-22 19:51     ` Helge Deller
  2019-11-24 10:20       ` Qais Yousef
  0 siblings, 1 reply; 33+ messages in thread
From: Helge Deller @ 2019-11-22 19:51 UTC (permalink / raw)
  To: Qais Yousef, James E.J. Bottomley
  Cc: Richard Fontana, Armijn Hemel, linux-parisc, linux-kernel,
	Thomas Gleixner, Greg Kroah-Hartman

On 20.11.19 12:09, Qais Yousef wrote:
> On 10/30/19 15:38, Qais Yousef wrote:
>> The core device API performs extra housekeeping bits that are missing
>> from directly calling cpu_up/down.
>>
>> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
>> serialization during LPM") for an example description of what might go
>> wrong.
>>
>> This also prepares to make cpu_up/down a private interface for anything
>> but the cpu subsystem.
>>
>> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
>> CC: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>> CC: Helge Deller <deller@gmx.de>
>> CC: Richard Fontana <rfontana@redhat.com>
>> CC: Armijn Hemel <armijn@tjaldur.nl>
>> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> CC: Thomas Gleixner <tglx@linutronix.de>
>> CC: linux-parisc@vger.kernel.org
>> CC: linux-kernel@vger.kernel.org
>> ---
>>
>> Couldn't compile test this one.
>>
>> I'm not confident that this is a correct patch to be honest. This __init
>> indicates we're booting the secondary cpus and that might be too early in the
>> process to use the core API..?
>
> Helge, James
>
> Do you have any comment on this? I have no means to test it and I'd
> appreciate if you can spin it through one of your systems.

I pulled your cpu-hp-cleanup branch from git://linux-arm.org/linux-qy
and compiled a 32- and a 64-bit parisc kernel.

I faced one compile warning:
linux-2.6/kernel/cpu.c: In function ‘hibernation_bringup_sleep_cpu’:
linux-2.6/kernel/cpu.c:1237:1: warning: control reaches end of non-void function [-Wreturn-type]

Other than that the 32- and 64-bit SMP kernel booted nicely.
You may add to the series:
Acked-by: Helge Deller <deller@gmx.de>   # parisc

Thanks,
Helge


>>
>>  arch/parisc/kernel/processor.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
>> index 13f771f74ee3..4dde5fe78f0c 100644
>> --- a/arch/parisc/kernel/processor.c
>> +++ b/arch/parisc/kernel/processor.c
>> @@ -212,7 +212,9 @@ static int __init processor_probe(struct parisc_device *dev)
>>  #ifdef CONFIG_SMP
>>  	if (cpuid) {
>>  		set_cpu_present(cpuid, true);
>> -		cpu_up(cpuid);
>> +		lock_device_hotplug();
>> +		device_online(get_cpu_device(cpuid));
>> +		unlock_device_hotplug();
>>  	}
>>  #endif
>>
>> --
>> 2.17.1
>>


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 06/12] parisc: Replace cpu_up/down with device_online/offline
  2019-11-22 19:51     ` Helge Deller
@ 2019-11-24 10:20       ` Qais Yousef
  0 siblings, 0 replies; 33+ messages in thread
From: Qais Yousef @ 2019-11-24 10:20 UTC (permalink / raw)
  To: Helge Deller
  Cc: James E.J. Bottomley, Richard Fontana, Armijn Hemel,
	linux-parisc, linux-kernel, Thomas Gleixner, Greg Kroah-Hartman

On 11/22/19 20:51, Helge Deller wrote:
> On 20.11.19 12:09, Qais Yousef wrote:
> > On 10/30/19 15:38, Qais Yousef wrote:
> >> The core device API performs extra housekeeping bits that are missing
> >> from directly calling cpu_up/down.
> >>
> >> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> >> serialization during LPM") for an example description of what might go
> >> wrong.
> >>
> >> This also prepares to make cpu_up/down a private interface for anything
> >> but the cpu subsystem.
> >>
> >> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> >> CC: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> >> CC: Helge Deller <deller@gmx.de>
> >> CC: Richard Fontana <rfontana@redhat.com>
> >> CC: Armijn Hemel <armijn@tjaldur.nl>
> >> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> CC: Thomas Gleixner <tglx@linutronix.de>
> >> CC: linux-parisc@vger.kernel.org
> >> CC: linux-kernel@vger.kernel.org
> >> ---
> >>
> >> Couldn't compile test this one.
> >>
> >> I'm not confident that this is a correct patch to be honest. This __init
> >> indicates we're booting the secondary cpus and that might be too early in the
> >> process to use the core API..?
> >
> > Helge, James
> >
> > Do you have any comment on this? I have no means to test it and I'd
> > appreciate if you can spin it through one of your systems.
> 
> I pulled your cpu-hp-cleanup branch from git://linux-arm.org/linux-qy
> and compiled a 32- and a 64-bit parisc kernel.
> 
> I faced one compile warning:
> linux-2.6/kernel/cpu.c: In function ‘hibernation_bringup_sleep_cpu’:
> linux-2.6/kernel/cpu.c:1237:1: warning: control reaches end of non-void function [-Wreturn-type]
> 
> Other than that the 32- and 64-bit SMP kernel booted nicely.
> You may add to the series:
> Acked-by: Helge Deller <deller@gmx.de>   # parisc

Great! Thanks for testing it and reporting the warning.

Cheers

--
Qais Yousef

> 
> Thanks,
> Helge
> 
> 
> >>
> >>  arch/parisc/kernel/processor.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
> >> index 13f771f74ee3..4dde5fe78f0c 100644
> >> --- a/arch/parisc/kernel/processor.c
> >> +++ b/arch/parisc/kernel/processor.c
> >> @@ -212,7 +212,9 @@ static int __init processor_probe(struct parisc_device *dev)
> >>  #ifdef CONFIG_SMP
> >>  	if (cpuid) {
> >>  		set_cpu_present(cpuid, true);
> >> -		cpu_up(cpuid);
> >> +		lock_device_hotplug();
> >> +		device_online(get_cpu_device(cpuid));
> >> +		unlock_device_hotplug();
> >>  	}
> >>  #endif
> >>
> >> --
> >> 2.17.1
> >>
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2019-11-24 10:20 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191030153837.18107-1-qais.yousef@arm.com>
2019-10-30 15:38 ` [PATCH 01/12] arm64: hibernate.c: create a new function to handle cpu_up(sleep_cpu) Qais Yousef
2019-11-19 22:31   ` Thomas Gleixner
2019-11-19 22:51     ` Qais Yousef
2019-11-19 23:01       ` Thomas Gleixner
2019-11-19 23:21         ` Qais Yousef
2019-10-30 15:38 ` [PATCH 02/12] x86: Replace cpu_up/down with devcie_online/offline Qais Yousef
2019-10-30 15:38 ` [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline Qais Yousef
2019-11-19  1:19   ` Michael Ellerman
2019-11-19  9:47     ` Qais Yousef
2019-10-30 15:38 ` [PATCH 04/12] ia64: Replace cpu_down with freeze_secondary_cpus Qais Yousef
2019-11-02  4:59   ` kbuild test robot
2019-11-19 22:21   ` Thomas Gleixner
2019-11-19 22:32     ` Qais Yousef
2019-11-19 22:59       ` Thomas Gleixner
2019-11-19 23:19         ` Qais Yousef
2019-11-20  8:46           ` Thomas Gleixner
2019-11-20 10:49             ` Qais Yousef
2019-10-30 15:38 ` [PATCH 05/12] sparc: Replace cpu_up/down with device_online/offline Qais Yousef
2019-10-30 19:33   ` David Miller
2019-10-30 15:38 ` [PATCH 06/12] parisc: " Qais Yousef
2019-11-20 11:09   ` Qais Yousef
2019-11-22 19:51     ` Helge Deller
2019-11-24 10:20       ` Qais Yousef
2019-10-30 15:38 ` [PATCH 07/12] driver: base: cpu: export device_online/offline Qais Yousef
2019-11-02 17:17   ` Greg Kroah-Hartman
2019-10-30 15:38 ` [PATCH 08/12] driver: xen: Replace cpu_up/down with device_online/offline Qais Yousef
2019-10-30 15:38 ` [PATCH 09/12] firmware: psci: " Qais Yousef
2019-10-30 15:38 ` [PATCH 10/12] torture: " Qais Yousef
2019-10-30 15:38 ` [PATCH 11/12] smp: Create a new function to bringup nonboot cpus online Qais Yousef
2019-11-19 22:42   ` Thomas Gleixner
2019-10-30 15:38 ` [PATCH 12/12] cpu: Hide cpu_up/down Qais Yousef
2019-11-19 22:25   ` Thomas Gleixner
2019-11-19 22:36     ` Qais Yousef

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).