linux-arm-kernel.lists.infradead.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 09/12] firmware: psci: Replace cpu_up/down with device_online/offline Qais Yousef
  1 sibling, 1 reply; 6+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Mark Rutland, linux-kernel, Steve Capper, Peter Zijlstra (Intel),
	Catalin Marinas, Daniel Lezcano, Pavankumar Kondeti,
	Zhenzhong Duan, Nicholas Piggin, Ingo Molnar, Richard Fontana,
	James Morse, Josh Poimboeuf, Jiri Kosina, Will Deacon,
	Qais Yousef, linux-arm-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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ 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>
  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
  1 sibling, 0 replies; 6+ messages in thread
From: Qais Yousef @ 2019-10-30 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman
  Cc: Mark Rutland, Lorenzo Pieralisi, Qais Yousef, 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ 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; 6+ messages in thread
From: Thomas Gleixner @ 2019-11-19 22:31 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Mark Rutland, Daniel Lezcano, Steve Capper,
	Peter Zijlstra (Intel),
	Greg Kroah-Hartman, Josh Poimboeuf, Pavankumar Kondeti,
	Zhenzhong Duan, Nicholas Piggin, linux-kernel, Richard Fontana,
	James Morse, Catalin Marinas, Jiri Kosina, Will Deacon,
	Ingo Molnar, linux-arm-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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ 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; 6+ messages in thread
From: Qais Yousef @ 2019-11-19 22:51 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Daniel Lezcano, Steve Capper,
	Peter Zijlstra (Intel),
	Greg Kroah-Hartman, Josh Poimboeuf, Pavankumar Kondeti,
	Zhenzhong Duan, Nicholas Piggin, linux-kernel, Richard Fontana,
	James Morse, Catalin Marinas, Jiri Kosina, Will Deacon,
	Ingo Molnar, linux-arm-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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ 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; 6+ messages in thread
From: Thomas Gleixner @ 2019-11-19 23:01 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Mark Rutland, Daniel Lezcano, Steve Capper,
	Peter Zijlstra (Intel),
	Greg Kroah-Hartman, Josh Poimboeuf, Pavankumar Kondeti,
	Zhenzhong Duan, Nicholas Piggin, linux-kernel, Richard Fontana,
	James Morse, Catalin Marinas, Jiri Kosina, Will Deacon,
	Ingo Molnar, linux-arm-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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ 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; 6+ messages in thread
From: Qais Yousef @ 2019-11-19 23:21 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Mark Rutland, Daniel Lezcano, Steve Capper,
	Peter Zijlstra (Intel),
	Greg Kroah-Hartman, Josh Poimboeuf, Pavankumar Kondeti,
	Zhenzhong Duan, Nicholas Piggin, linux-kernel, Richard Fontana,
	James Morse, Catalin Marinas, Jiri Kosina, Will Deacon,
	Ingo Molnar, linux-arm-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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

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

Thread overview: 6+ 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 09/12] firmware: psci: Replace cpu_up/down with device_online/offline 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).