* How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
@ 2016-12-21 16:54 ` Sudeep Holla
0 siblings, 0 replies; 21+ messages in thread
From: Sudeep Holla @ 2016-12-21 16:54 UTC (permalink / raw)
To: linux-arm-kernel
On 21/12/16 11:37, Jisheng Zhang wrote:
> Hi all,
>
> I'm not sure this is a bug, when wake up from s2ram, I could get something
> like:
>
> [ 313.271464] Enabling non-boot CPUs ...
> [ 313.271551] CPU1: Booted secondary processor
> [ 313.271556] Detected VIPT I-cache on CPU1
> [ 313.301378] cache: parent cpu1 should not be sleeping
> [ 313.301504] CPU1 is up
> [ 313.301582] CPU2: Booted secondary processor
> [ 313.301585] Detected VIPT I-cache on CPU2
> [ 313.331485] cache: parent cpu2 should not be sleeping
> [ 313.331605] CPU2 is up
> [ 313.331683] CPU3: Booted secondary processor
> [ 313.331686] Detected VIPT I-cache on CPU3
> [ 313.361599] cache: parent cpu3 should not be sleeping
> [ 313.361719] CPU3 is up
>
> This is because we call cpu_device_create() when secondary cpu is brought
> online, the cpu_cache device's parent device: cpu device isn't already
> resumed, all device resume will resume after secondary cores are brought
> up.
>
> What's the elegant solution to remove this warning msg?
>
It is not a serious warning as you can see a comment in the code:
"/*
* This is a fib. But we'll allow new children to be added below
* a resumed device, even if the device hasn't been completed yet.
*/"
But if we think it needs to be removed, I have something like below in
my mind. I am not sure if this is hacky or not(completely untested, not
even compiled).
Regards,
Sudeep
-->8
diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
index 4c28e1a09786..2a5c04377adf 100644
--- i/drivers/base/cpu.c
+++ w/drivers/base/cpu.c
@@ -344,6 +344,14 @@ static int cpu_uevent(struct device *dev, struct
kobj_uevent_env *env)
}
#endif
+static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
+{
+ struct device *cpu_dev = get_cpu_device(cpu);
+
+ if(cpu_dev)
+ cpu_dev->power.is_prepared = false;
+ return 0;
+}
/*
* register_cpu - Setup a sysfs device for a CPU.
* @cpu - cpu->hotpluggable field set to 1 will generate a control file in
@@ -377,7 +385,9 @@ int register_cpu(struct cpu *cpu, int num)
per_cpu(cpu_sys_devices, num) = &cpu->dev;
register_cpu_under_node(num, cpu_to_node(num));
- return 0;
+ return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
+ "base/cpu/dev_pm:prepare",
+ cpu_dev_pm_unset_is_prepared, NULL);
}
struct device *get_cpu_device(unsigned cpu)
diff --git i/include/linux/cpuhotplug.h w/include/linux/cpuhotplug.h
index 2ab7bf53d529..5bfe3c1aa148 100644
--- i/include/linux/cpuhotplug.h
+++ w/include/linux/cpuhotplug.h
@@ -51,6 +51,7 @@ enum cpuhp_state {
CPUHP_SLAB_PREPARE,
CPUHP_MD_RAID5_PREPARE,
CPUHP_RCUTREE_PREP,
+ CPUHP_CPUDEV_PM_PREPARE,
CPUHP_CPUIDLE_COUPLED_PREPARE,
CPUHP_POWERPC_PMAC_PREPARE,
CPUHP_POWERPC_MMU_CTX_PREPARE,
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
2016-12-21 16:54 ` Sudeep Holla
@ 2016-12-22 7:48 ` Jisheng Zhang
-1 siblings, 0 replies; 21+ messages in thread
From: Jisheng Zhang @ 2016-12-22 7:48 UTC (permalink / raw)
To: Sudeep Holla; +Cc: linux-arm-kernel, linux-pm, Rafael J. Wysocki
On Wed, 21 Dec 2016 16:54:22 +0000 Sudeep Holla wrote:
> On 21/12/16 11:37, Jisheng Zhang wrote:
> > Hi all,
> >
> > I'm not sure this is a bug, when wake up from s2ram, I could get something
> > like:
> >
> > [ 313.271464] Enabling non-boot CPUs ...
> > [ 313.271551] CPU1: Booted secondary processor
> > [ 313.271556] Detected VIPT I-cache on CPU1
> > [ 313.301378] cache: parent cpu1 should not be sleeping
> > [ 313.301504] CPU1 is up
> > [ 313.301582] CPU2: Booted secondary processor
> > [ 313.301585] Detected VIPT I-cache on CPU2
> > [ 313.331485] cache: parent cpu2 should not be sleeping
> > [ 313.331605] CPU2 is up
> > [ 313.331683] CPU3: Booted secondary processor
> > [ 313.331686] Detected VIPT I-cache on CPU3
> > [ 313.361599] cache: parent cpu3 should not be sleeping
> > [ 313.361719] CPU3 is up
> >
> > This is because we call cpu_device_create() when secondary cpu is brought
> > online, the cpu_cache device's parent device: cpu device isn't already
> > resumed, all device resume will resume after secondary cores are brought
> > up.
> >
> > What's the elegant solution to remove this warning msg?
> >
>
> It is not a serious warning as you can see a comment in the code:
> "/*
> * This is a fib. But we'll allow new children to be added below
> * a resumed device, even if the device hasn't been completed yet.
> */"
>
Great, make sense, I agree with "this is not a serious warn msg".
Thanks for the help,
Jisheng
> But if we think it needs to be removed, I have something like below in
> my mind. I am not sure if this is hacky or not(completely untested, not
> even compiled).
>
> Regards,
> Sudeep
>
> -->8
>
> diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
> index 4c28e1a09786..2a5c04377adf 100644
> --- i/drivers/base/cpu.c
> +++ w/drivers/base/cpu.c
> @@ -344,6 +344,14 @@ static int cpu_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> }
> #endif
>
> +static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
> +{
> + struct device *cpu_dev = get_cpu_device(cpu);
> +
> + if(cpu_dev)
> + cpu_dev->power.is_prepared = false;
> + return 0;
> +}
> /*
> * register_cpu - Setup a sysfs device for a CPU.
> * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
> @@ -377,7 +385,9 @@ int register_cpu(struct cpu *cpu, int num)
> per_cpu(cpu_sys_devices, num) = &cpu->dev;
> register_cpu_under_node(num, cpu_to_node(num));
>
> - return 0;
> + return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
> + "base/cpu/dev_pm:prepare",
> + cpu_dev_pm_unset_is_prepared, NULL);
> }
>
> struct device *get_cpu_device(unsigned cpu)
> diff --git i/include/linux/cpuhotplug.h w/include/linux/cpuhotplug.h
> index 2ab7bf53d529..5bfe3c1aa148 100644
> --- i/include/linux/cpuhotplug.h
> +++ w/include/linux/cpuhotplug.h
> @@ -51,6 +51,7 @@ enum cpuhp_state {
> CPUHP_SLAB_PREPARE,
> CPUHP_MD_RAID5_PREPARE,
> CPUHP_RCUTREE_PREP,
> + CPUHP_CPUDEV_PM_PREPARE,
> CPUHP_CPUIDLE_COUPLED_PREPARE,
> CPUHP_POWERPC_PMAC_PREPARE,
> CPUHP_POWERPC_MMU_CTX_PREPARE,
^ permalink raw reply [flat|nested] 21+ messages in thread* How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
@ 2016-12-22 7:48 ` Jisheng Zhang
0 siblings, 0 replies; 21+ messages in thread
From: Jisheng Zhang @ 2016-12-22 7:48 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 21 Dec 2016 16:54:22 +0000 Sudeep Holla wrote:
> On 21/12/16 11:37, Jisheng Zhang wrote:
> > Hi all,
> >
> > I'm not sure this is a bug, when wake up from s2ram, I could get something
> > like:
> >
> > [ 313.271464] Enabling non-boot CPUs ...
> > [ 313.271551] CPU1: Booted secondary processor
> > [ 313.271556] Detected VIPT I-cache on CPU1
> > [ 313.301378] cache: parent cpu1 should not be sleeping
> > [ 313.301504] CPU1 is up
> > [ 313.301582] CPU2: Booted secondary processor
> > [ 313.301585] Detected VIPT I-cache on CPU2
> > [ 313.331485] cache: parent cpu2 should not be sleeping
> > [ 313.331605] CPU2 is up
> > [ 313.331683] CPU3: Booted secondary processor
> > [ 313.331686] Detected VIPT I-cache on CPU3
> > [ 313.361599] cache: parent cpu3 should not be sleeping
> > [ 313.361719] CPU3 is up
> >
> > This is because we call cpu_device_create() when secondary cpu is brought
> > online, the cpu_cache device's parent device: cpu device isn't already
> > resumed, all device resume will resume after secondary cores are brought
> > up.
> >
> > What's the elegant solution to remove this warning msg?
> >
>
> It is not a serious warning as you can see a comment in the code:
> "/*
> * This is a fib. But we'll allow new children to be added below
> * a resumed device, even if the device hasn't been completed yet.
> */"
>
Great, make sense, I agree with "this is not a serious warn msg".
Thanks for the help,
Jisheng
> But if we think it needs to be removed, I have something like below in
> my mind. I am not sure if this is hacky or not(completely untested, not
> even compiled).
>
> Regards,
> Sudeep
>
> -->8
>
> diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
> index 4c28e1a09786..2a5c04377adf 100644
> --- i/drivers/base/cpu.c
> +++ w/drivers/base/cpu.c
> @@ -344,6 +344,14 @@ static int cpu_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> }
> #endif
>
> +static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
> +{
> + struct device *cpu_dev = get_cpu_device(cpu);
> +
> + if(cpu_dev)
> + cpu_dev->power.is_prepared = false;
> + return 0;
> +}
> /*
> * register_cpu - Setup a sysfs device for a CPU.
> * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
> @@ -377,7 +385,9 @@ int register_cpu(struct cpu *cpu, int num)
> per_cpu(cpu_sys_devices, num) = &cpu->dev;
> register_cpu_under_node(num, cpu_to_node(num));
>
> - return 0;
> + return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
> + "base/cpu/dev_pm:prepare",
> + cpu_dev_pm_unset_is_prepared, NULL);
> }
>
> struct device *get_cpu_device(unsigned cpu)
> diff --git i/include/linux/cpuhotplug.h w/include/linux/cpuhotplug.h
> index 2ab7bf53d529..5bfe3c1aa148 100644
> --- i/include/linux/cpuhotplug.h
> +++ w/include/linux/cpuhotplug.h
> @@ -51,6 +51,7 @@ enum cpuhp_state {
> CPUHP_SLAB_PREPARE,
> CPUHP_MD_RAID5_PREPARE,
> CPUHP_RCUTREE_PREP,
> + CPUHP_CPUDEV_PM_PREPARE,
> CPUHP_CPUIDLE_COUPLED_PREPARE,
> CPUHP_POWERPC_PMAC_PREPARE,
> CPUHP_POWERPC_MMU_CTX_PREPARE,
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
2016-12-21 16:54 ` Sudeep Holla
@ 2018-08-20 17:46 ` Steve Longerbeam
-1 siblings, 0 replies; 21+ messages in thread
From: Steve Longerbeam @ 2018-08-20 17:46 UTC (permalink / raw)
To: Sudeep Holla, Jisheng Zhang, linux-arm-kernel, linux-pm,
Rafael J. Wysocki
Hi Sudeep,
On 12/21/2016 08:54 AM, Sudeep Holla wrote:
>
> On 21/12/16 11:37, Jisheng Zhang wrote:
>> Hi all,
>>
>> I'm not sure this is a bug, when wake up from s2ram, I could get something
>> like:
>>
>> [ 313.271464] Enabling non-boot CPUs ...
>> [ 313.271551] CPU1: Booted secondary processor
>> [ 313.271556] Detected VIPT I-cache on CPU1
>> [ 313.301378] cache: parent cpu1 should not be sleeping
>> [ 313.301504] CPU1 is up
>> [ 313.301582] CPU2: Booted secondary processor
>> [ 313.301585] Detected VIPT I-cache on CPU2
>> [ 313.331485] cache: parent cpu2 should not be sleeping
>> [ 313.331605] CPU2 is up
>> [ 313.331683] CPU3: Booted secondary processor
>> [ 313.331686] Detected VIPT I-cache on CPU3
>> [ 313.361599] cache: parent cpu3 should not be sleeping
>> [ 313.361719] CPU3 is up
>>
>> This is because we call cpu_device_create() when secondary cpu is brought
>> online, the cpu_cache device's parent device: cpu device isn't already
>> resumed, all device resume will resume after secondary cores are brought
>> up.
>>
>> What's the elegant solution to remove this warning msg?
>>
> It is not a serious warning as you can see a comment in the code:
> "/*
> * This is a fib. But we'll allow new children to be added below
> * a resumed device, even if the device hasn't been completed yet.
> */"
>
> But if we think it needs to be removed, I have something like below in
> my mind. I am not sure if this is hacky or not(completely untested, not
> even compiled).
This is an old thread, but this warning still exists in the code and
still occurs
on resume from suspend-to-ram, as of 4.18-rc2.
I have tested the below patch on a Renesas Salvator-X ES2.0 and it does
indeed remove this warning on S2RAM resume.
I agree it is not serious, but the text of the warning does not sound
harmless.
So IMHO the text should be modified or the below patch should be submitted
for review/merge.
Thanks,
Steve
> Regards,
> Sudeep
>
> -->8
>
> diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
> index 4c28e1a09786..2a5c04377adf 100644
> --- i/drivers/base/cpu.c
> +++ w/drivers/base/cpu.c
> @@ -344,6 +344,14 @@ static int cpu_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> }
> #endif
>
> +static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
> +{
> + struct device *cpu_dev = get_cpu_device(cpu);
> +
> + if(cpu_dev)
> + cpu_dev->power.is_prepared = false;
> + return 0;
> +}
> /*
> * register_cpu - Setup a sysfs device for a CPU.
> * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
> @@ -377,7 +385,9 @@ int register_cpu(struct cpu *cpu, int num)
> per_cpu(cpu_sys_devices, num) = &cpu->dev;
> register_cpu_under_node(num, cpu_to_node(num));
>
> - return 0;
> + return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
> + "base/cpu/dev_pm:prepare",
> + cpu_dev_pm_unset_is_prepared, NULL);
> }
>
> struct device *get_cpu_device(unsigned cpu)
> diff --git i/include/linux/cpuhotplug.h w/include/linux/cpuhotplug.h
> index 2ab7bf53d529..5bfe3c1aa148 100644
> --- i/include/linux/cpuhotplug.h
> +++ w/include/linux/cpuhotplug.h
> @@ -51,6 +51,7 @@ enum cpuhp_state {
> CPUHP_SLAB_PREPARE,
> CPUHP_MD_RAID5_PREPARE,
> CPUHP_RCUTREE_PREP,
> + CPUHP_CPUDEV_PM_PREPARE,
> CPUHP_CPUIDLE_COUPLED_PREPARE,
> CPUHP_POWERPC_PMAC_PREPARE,
> CPUHP_POWERPC_MMU_CTX_PREPARE,
>
> _______________________________________________
> 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] 21+ messages in thread* How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
@ 2018-08-20 17:46 ` Steve Longerbeam
0 siblings, 0 replies; 21+ messages in thread
From: Steve Longerbeam @ 2018-08-20 17:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sudeep,
On 12/21/2016 08:54 AM, Sudeep Holla wrote:
>
> On 21/12/16 11:37, Jisheng Zhang wrote:
>> Hi all,
>>
>> I'm not sure this is a bug, when wake up from s2ram, I could get something
>> like:
>>
>> [ 313.271464] Enabling non-boot CPUs ...
>> [ 313.271551] CPU1: Booted secondary processor
>> [ 313.271556] Detected VIPT I-cache on CPU1
>> [ 313.301378] cache: parent cpu1 should not be sleeping
>> [ 313.301504] CPU1 is up
>> [ 313.301582] CPU2: Booted secondary processor
>> [ 313.301585] Detected VIPT I-cache on CPU2
>> [ 313.331485] cache: parent cpu2 should not be sleeping
>> [ 313.331605] CPU2 is up
>> [ 313.331683] CPU3: Booted secondary processor
>> [ 313.331686] Detected VIPT I-cache on CPU3
>> [ 313.361599] cache: parent cpu3 should not be sleeping
>> [ 313.361719] CPU3 is up
>>
>> This is because we call cpu_device_create() when secondary cpu is brought
>> online, the cpu_cache device's parent device: cpu device isn't already
>> resumed, all device resume will resume after secondary cores are brought
>> up.
>>
>> What's the elegant solution to remove this warning msg?
>>
> It is not a serious warning as you can see a comment in the code:
> "/*
> * This is a fib. But we'll allow new children to be added below
> * a resumed device, even if the device hasn't been completed yet.
> */"
>
> But if we think it needs to be removed, I have something like below in
> my mind. I am not sure if this is hacky or not(completely untested, not
> even compiled).
This is an old thread, but this warning still exists in the code and
still occurs
on resume from suspend-to-ram, as of 4.18-rc2.
I have tested the below patch on a Renesas Salvator-X ES2.0 and it does
indeed remove this warning on S2RAM resume.
I agree it is not serious, but the text of the warning does not sound
harmless.
So IMHO the text should be modified or the below patch should be submitted
for review/merge.
Thanks,
Steve
> Regards,
> Sudeep
>
> -->8
>
> diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
> index 4c28e1a09786..2a5c04377adf 100644
> --- i/drivers/base/cpu.c
> +++ w/drivers/base/cpu.c
> @@ -344,6 +344,14 @@ static int cpu_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> }
> #endif
>
> +static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
> +{
> + struct device *cpu_dev = get_cpu_device(cpu);
> +
> + if(cpu_dev)
> + cpu_dev->power.is_prepared = false;
> + return 0;
> +}
> /*
> * register_cpu - Setup a sysfs device for a CPU.
> * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
> @@ -377,7 +385,9 @@ int register_cpu(struct cpu *cpu, int num)
> per_cpu(cpu_sys_devices, num) = &cpu->dev;
> register_cpu_under_node(num, cpu_to_node(num));
>
> - return 0;
> + return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
> + "base/cpu/dev_pm:prepare",
> + cpu_dev_pm_unset_is_prepared, NULL);
> }
>
> struct device *get_cpu_device(unsigned cpu)
> diff --git i/include/linux/cpuhotplug.h w/include/linux/cpuhotplug.h
> index 2ab7bf53d529..5bfe3c1aa148 100644
> --- i/include/linux/cpuhotplug.h
> +++ w/include/linux/cpuhotplug.h
> @@ -51,6 +51,7 @@ enum cpuhp_state {
> CPUHP_SLAB_PREPARE,
> CPUHP_MD_RAID5_PREPARE,
> CPUHP_RCUTREE_PREP,
> + CPUHP_CPUDEV_PM_PREPARE,
> CPUHP_CPUIDLE_COUPLED_PREPARE,
> CPUHP_POWERPC_PMAC_PREPARE,
> CPUHP_POWERPC_MMU_CTX_PREPARE,
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
2016-12-21 16:54 ` Sudeep Holla
@ 2018-10-02 17:07 ` Eugeniu Rosca
-1 siblings, 0 replies; 21+ messages in thread
From: Eugeniu Rosca @ 2018-10-02 17:07 UTC (permalink / raw)
To: Sudeep Holla
Cc: Jisheng Zhang, Steve Longerbeam, linux-pm, Eugeniu Rosca,
Rafael J. Wysocki, Eugeniu Rosca, linux-arm-kernel
Hello Sudeep,
I too experience the "cache: parent cpuN should not be sleeping" and
confirm these are healed with your patch. Would it be possible to have
the patch submitted for review?
Thank you very much,
Eugeniu.
^ permalink raw reply [flat|nested] 21+ messages in thread
* How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
@ 2018-10-02 17:07 ` Eugeniu Rosca
0 siblings, 0 replies; 21+ messages in thread
From: Eugeniu Rosca @ 2018-10-02 17:07 UTC (permalink / raw)
To: linux-arm-kernel
Hello Sudeep,
I too experience the "cache: parent cpuN should not be sleeping" and
confirm these are healed with your patch. Would it be possible to have
the patch submitted for review?
Thank you very much,
Eugeniu.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
2018-10-02 17:07 ` Eugeniu Rosca
@ 2019-01-25 13:07 ` Eugeniu Rosca
-1 siblings, 0 replies; 21+ messages in thread
From: Eugeniu Rosca @ 2019-01-25 13:07 UTC (permalink / raw)
To: Sudeep Holla
Cc: Jisheng Zhang, Steve Longerbeam, linux-pm, Eugeniu Rosca,
Rafael J. Wysocki, Eugeniu Rosca, linux-arm-kernel,
Joshua Frkuska
Hi Sudeep,
On Tue, Oct 02, 2018 at 07:07:54PM +0200, Eugeniu Rosca wrote:
> Hello Sudeep,
>
> I too experience the "cache: parent cpuN should not be sleeping" and
> confirm these are healed with your patch. Would it be possible to have
> the patch submitted for review?
Doing some s2ram testing on R-Car H3 Salvator-X using
v5.0-rc3-130-gd73aba1115cf, I am still able to reproduce these warnings.
Is there any chance you push the patch to the mailing list? If not, can
it be taken with a different authorship by mentioning this discussion
thread as source of inspiration?
> Thank you very much,
> Eugeniu.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
@ 2019-01-25 13:07 ` Eugeniu Rosca
0 siblings, 0 replies; 21+ messages in thread
From: Eugeniu Rosca @ 2019-01-25 13:07 UTC (permalink / raw)
To: Sudeep Holla
Cc: Jisheng Zhang, Steve Longerbeam, linux-pm, Eugeniu Rosca,
Rafael J. Wysocki, Eugeniu Rosca, linux-arm-kernel,
Joshua Frkuska
Hi Sudeep,
On Tue, Oct 02, 2018 at 07:07:54PM +0200, Eugeniu Rosca wrote:
> Hello Sudeep,
>
> I too experience the "cache: parent cpuN should not be sleeping" and
> confirm these are healed with your patch. Would it be possible to have
> the patch submitted for review?
Doing some s2ram testing on R-Car H3 Salvator-X using
v5.0-rc3-130-gd73aba1115cf, I am still able to reproduce these warnings.
Is there any chance you push the patch to the mailing list? If not, can
it be taken with a different authorship by mentioning this discussion
thread as source of inspiration?
> Thank you very much,
> Eugeniu.
_______________________________________________
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] 21+ messages in thread
* [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-01-25 13:07 ` Eugeniu Rosca
(?)
@ 2019-01-25 15:09 ` Sudeep Holla
2019-01-27 13:57 ` Eugeniu Rosca
2019-01-30 23:48 ` Rafael J. Wysocki
-1 siblings, 2 replies; 21+ messages in thread
From: Sudeep Holla @ 2019-01-25 15:09 UTC (permalink / raw)
To: linux-pm, linux-kernel
Cc: Sudeep Holla, Jisheng Zhang, Steve Longerbeam, Eugeniu Rosca,
Joshua Frkuska, Greg Kroah-Hartman, Rafael J. Wysocki
The sysfs for the cpu caches are managed by adding devices with cpu
as the parent in cpu_device_create() when secondary cpu is brought
onlin. Generally when the secondary CPUs are hotplugged back is as part
of resume from suspend-to-ram, we call cpu_device_create() from the cpu
hotplug state machine while the cpu device associated with that CPU is
not yet ready to be resumed as the device_resume() call happens bit later.
It's not really needed to set the flag is_prepared for cpu devices are
they are mostly pseudo device and hotplug framework deals with state
machine and not managed through the cpu device.
This often results in annoying warning when resuming:
Enabling non-boot CPUs ...
CPU1: Booted secondary processor
cache: parent cpu1 should not be sleeping
CPU1 is up
CPU2: Booted secondary processor
cache: parent cpu2 should not be sleeping
CPU2 is up
.... and so on.
Just fix the warning by updating the device state quite early.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Reported-by: Jisheng Zhang <jszhang@marvell.com>
Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/base/cpu.c | 20 +++++++++++++++++++-
include/linux/cpuhotplug.h | 1 +
2 files changed, 20 insertions(+), 1 deletion(-)
Hi Rafael,
This is getting reported for quite some time. Let me know if you have
better solution to fix this harmless yet annoying warnings during system
resume.
Regards,
Sudeep
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index eb9443d5bae1..ae0403528bff 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -80,6 +80,8 @@ void unregister_cpu(struct cpu *cpu)
device_unregister(&cpu->dev);
per_cpu(cpu_sys_devices, logical_cpu) = NULL;
+
+ cpuhp_remove_state_nocalls(CPUHP_CPUDEV_PM_PREPARE);
return;
}
@@ -355,6 +357,20 @@ static int cpu_uevent(struct device *dev, struct kobj_uevent_env *env)
}
#endif
+static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
+{
+ struct device *cpu_dev = get_cpu_device(cpu);
+
+ /*
+ * device_resume sets this for cpu_dev bit later but the child
+ * devices are resumes in the cpu hotplug state machine much
+ * before device_resume is called.
+ */
+ if (cpu_dev)
+ cpu_dev->power.is_prepared = false;
+
+ return 0;
+}
/*
* register_cpu - Setup a sysfs device for a CPU.
* @cpu - cpu->hotpluggable field set to 1 will generate a control file in
@@ -392,7 +408,9 @@ int register_cpu(struct cpu *cpu, int num)
dev_pm_qos_expose_latency_limit(&cpu->dev,
PM_QOS_RESUME_LATENCY_NO_CONSTRAINT);
- return 0;
+ return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
+ "base/cpu/dev_pm:prepare",
+ cpu_dev_pm_unset_is_prepared, NULL);
}
struct device *get_cpu_device(unsigned cpu)
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index fd586d0301e7..2ecb4c9370ce 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -69,6 +69,7 @@ enum cpuhp_state {
CPUHP_SLAB_PREPARE,
CPUHP_MD_RAID5_PREPARE,
CPUHP_RCUTREE_PREP,
+ CPUHP_CPUDEV_PM_PREPARE,
CPUHP_CPUIDLE_COUPLED_PREPARE,
CPUHP_POWERPC_PMAC_PREPARE,
CPUHP_POWERPC_MMU_CTX_PREPARE,
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-01-25 15:09 ` [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed Sudeep Holla
@ 2019-01-27 13:57 ` Eugeniu Rosca
2019-01-30 23:48 ` Rafael J. Wysocki
1 sibling, 0 replies; 21+ messages in thread
From: Eugeniu Rosca @ 2019-01-27 13:57 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Joshua Frkuska, Greg Kroah-Hartman, Rafael J. Wysocki,
Eugeniu Rosca, Eugeniu Rosca
FWIW the "cache: parent cpuN should not be sleeping" warnings no longer
appear during s2ram procedure on R-Car H3-ES20 Salvator-X with this patch.
The kernel version is v5.0-rc3-241-g7c2614bf7a1f.
The test procedure is:
root@salvator-x:~# modprobe i2c-dev
root@salvator-x:~# i2cset -f -y 7 0x30 0x20 0x0F
root@salvator-x:~# echo deep > /sys/power/mem_sleep
root@salvator-x:~# echo mem > /sys/power/state
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-01-25 15:09 ` [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed Sudeep Holla
2019-01-27 13:57 ` Eugeniu Rosca
@ 2019-01-30 23:48 ` Rafael J. Wysocki
2019-01-31 16:05 ` Sudeep Holla
1 sibling, 1 reply; 21+ messages in thread
From: Rafael J. Wysocki @ 2019-01-30 23:48 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Eugeniu Rosca, Joshua Frkuska, Greg Kroah-Hartman
On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> The sysfs for the cpu caches are managed by adding devices with cpu
> as the parent in cpu_device_create() when secondary cpu is brought
> onlin. Generally when the secondary CPUs are hotplugged back is as part
> of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> hotplug state machine while the cpu device associated with that CPU is
> not yet ready to be resumed as the device_resume() call happens bit later.
> It's not really needed to set the flag is_prepared for cpu devices are
> they are mostly pseudo device and hotplug framework deals with state
> machine and not managed through the cpu device.
>
> This often results in annoying warning when resuming:
> Enabling non-boot CPUs ...
> CPU1: Booted secondary processor
> cache: parent cpu1 should not be sleeping
> CPU1 is up
> CPU2: Booted secondary processor
> cache: parent cpu2 should not be sleeping
> CPU2 is up
> .... and so on.
>
> Just fix the warning by updating the device state quite early.
>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Reported-by: Jisheng Zhang <jszhang@marvell.com>
> Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/base/cpu.c | 20 +++++++++++++++++++-
> include/linux/cpuhotplug.h | 1 +
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> Hi Rafael,
>
> This is getting reported for quite some time. Let me know if you have
> better solution to fix this harmless yet annoying warnings during system
> resume.
I'd rather have a flag in struct dev_pm_info that will cause the message to
be suppressed if set.
It could be used for other purposes too then in princple (like skipping the
creation of empty "power" attr groups in sysfs for devices that don't
need them etc.).
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index eb9443d5bae1..ae0403528bff 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -80,6 +80,8 @@ void unregister_cpu(struct cpu *cpu)
>
> device_unregister(&cpu->dev);
> per_cpu(cpu_sys_devices, logical_cpu) = NULL;
> +
> + cpuhp_remove_state_nocalls(CPUHP_CPUDEV_PM_PREPARE);
> return;
> }
>
> @@ -355,6 +357,20 @@ static int cpu_uevent(struct device *dev, struct kobj_uevent_env *env)
> }
> #endif
>
> +static int cpu_dev_pm_unset_is_prepared(unsigned int cpu)
> +{
> + struct device *cpu_dev = get_cpu_device(cpu);
> +
> + /*
> + * device_resume sets this for cpu_dev bit later but the child
> + * devices are resumes in the cpu hotplug state machine much
> + * before device_resume is called.
> + */
> + if (cpu_dev)
> + cpu_dev->power.is_prepared = false;
> +
> + return 0;
> +}
> /*
> * register_cpu - Setup a sysfs device for a CPU.
> * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
> @@ -392,7 +408,9 @@ int register_cpu(struct cpu *cpu, int num)
> dev_pm_qos_expose_latency_limit(&cpu->dev,
> PM_QOS_RESUME_LATENCY_NO_CONSTRAINT);
>
> - return 0;
> + return cpuhp_setup_state_nocalls(CPUHP_CPUDEV_PM_PREPARE,
> + "base/cpu/dev_pm:prepare",
> + cpu_dev_pm_unset_is_prepared, NULL);
> }
>
> struct device *get_cpu_device(unsigned cpu)
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index fd586d0301e7..2ecb4c9370ce 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -69,6 +69,7 @@ enum cpuhp_state {
> CPUHP_SLAB_PREPARE,
> CPUHP_MD_RAID5_PREPARE,
> CPUHP_RCUTREE_PREP,
> + CPUHP_CPUDEV_PM_PREPARE,
> CPUHP_CPUIDLE_COUPLED_PREPARE,
> CPUHP_POWERPC_PMAC_PREPARE,
> CPUHP_POWERPC_MMU_CTX_PREPARE,
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-01-30 23:48 ` Rafael J. Wysocki
@ 2019-01-31 16:05 ` Sudeep Holla
2019-02-04 15:37 ` Sudeep Holla
0 siblings, 1 reply; 21+ messages in thread
From: Sudeep Holla @ 2019-01-31 16:05 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Eugeniu Rosca, Joshua Frkuska, Greg Kroah-Hartman, Sudeep Holla
On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote:
> On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> > The sysfs for the cpu caches are managed by adding devices with cpu
> > as the parent in cpu_device_create() when secondary cpu is brought
> > onlin. Generally when the secondary CPUs are hotplugged back is as part
> > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > hotplug state machine while the cpu device associated with that CPU is
> > not yet ready to be resumed as the device_resume() call happens bit later.
> > It's not really needed to set the flag is_prepared for cpu devices are
> > they are mostly pseudo device and hotplug framework deals with state
> > machine and not managed through the cpu device.
> >
> > This often results in annoying warning when resuming:
> > Enabling non-boot CPUs ...
> > CPU1: Booted secondary processor
> > cache: parent cpu1 should not be sleeping
> > CPU1 is up
> > CPU2: Booted secondary processor
> > cache: parent cpu2 should not be sleeping
> > CPU2 is up
> > .... and so on.
> >
> > Just fix the warning by updating the device state quite early.
> >
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Reported-by: Jisheng Zhang <jszhang@marvell.com>
> > Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > drivers/base/cpu.c | 20 +++++++++++++++++++-
> > include/linux/cpuhotplug.h | 1 +
> > 2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > Hi Rafael,
> >
> > This is getting reported for quite some time. Let me know if you have
> > better solution to fix this harmless yet annoying warnings during system
> > resume.
>
> I'd rather have a flag in struct dev_pm_info that will cause the message to
> be suppressed if set.
>
> It could be used for other purposes too then in princple (like skipping the
> creation of empty "power" attr groups in sysfs for devices that don't
> need them etc.).
>
Thanks for the suggestion. I did quick hack and came up with something
below. I wanted to run through you once before I materialise it into
a formal patch to check if I understood your suggestion correctly.
We can move no_pm_required outside dev_pm_info struct and rename with
any better names.
-->8
diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
index eb9443d5bae1..b61f9772ed33 100644
--- i/drivers/base/cpu.c
+++ w/drivers/base/cpu.c
@@ -379,6 +379,7 @@ int register_cpu(struct cpu *cpu, int num)
cpu->dev.bus->uevent = cpu_uevent;
#endif
cpu->dev.groups = common_cpu_attr_groups;
+ cpu->dev.power.no_pm_required = true;
if (cpu->hotpluggable)
cpu->dev.groups = hotplugable_cpu_attr_groups;
error = device_register(&cpu->dev);
@@ -427,6 +428,7 @@ __cpu_device_create(struct device *parent, void *drvdata,
dev->parent = parent;
dev->groups = groups;
dev->release = device_create_release;
+ dev->power.no_pm_required = true;
dev_set_drvdata(dev, drvdata);
retval = kobject_set_name_vargs(&dev->kobj, fmt, args);
diff --git i/drivers/base/power/main.c w/drivers/base/power/main.c
index 0992e67e862b..ed1b133f73db 100644
--- i/drivers/base/power/main.c
+++ w/drivers/base/power/main.c
@@ -124,6 +124,8 @@ void device_pm_unlock(void)
*/
void device_pm_add(struct device *dev)
{
+ if (dev->power.no_pm_required)
+ return;
pr_debug("PM: Adding info for %s:%s\n",
dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
device_pm_check_callbacks(dev);
@@ -142,6 +144,8 @@ void device_pm_add(struct device *dev)
*/
void device_pm_remove(struct device *dev)
{
+ if (dev->power.no_pm_required)
+ return;
pr_debug("PM: Removing info for %s:%s\n",
dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
complete_all(&dev->power.completion);
diff --git i/drivers/base/power/sysfs.c w/drivers/base/power/sysfs.c
index d713738ce796..54c1bfec396e 100644
--- i/drivers/base/power/sysfs.c
+++ w/drivers/base/power/sysfs.c
@@ -648,6 +648,9 @@ int dpm_sysfs_add(struct device *dev)
{
int rc;
+ if (dev->power.no_pm_required)
+ return 0;
+
rc = sysfs_create_group(&dev->kobj, &pm_attr_group);
if (rc)
return rc;
diff --git i/include/linux/pm.h w/include/linux/pm.h
index 0bd9de116826..300ab9f0b858 100644
--- i/include/linux/pm.h
+++ w/include/linux/pm.h
@@ -592,6 +592,7 @@ struct dev_pm_info {
bool is_suspended:1; /* Ditto */
bool is_noirq_suspended:1;
bool is_late_suspended:1;
+ bool no_pm_required:1;
bool early_init:1; /* Owned by the PM core */
bool direct_complete:1; /* Owned by the PM core */
u32 driver_flags;
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-01-31 16:05 ` Sudeep Holla
@ 2019-02-04 15:37 ` Sudeep Holla
2019-02-04 15:44 ` Greg Kroah-Hartman
2019-02-06 10:31 ` Rafael J. Wysocki
0 siblings, 2 replies; 21+ messages in thread
From: Sudeep Holla @ 2019-02-04 15:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Eugeniu Rosca, Joshua Frkuska, Sudeep Holla, Greg Kroah-Hartman
On Thu, Jan 31, 2019 at 04:05:59PM +0000, Sudeep Holla wrote:
> On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote:
> > On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> > > The sysfs for the cpu caches are managed by adding devices with cpu
> > > as the parent in cpu_device_create() when secondary cpu is brought
> > > onlin. Generally when the secondary CPUs are hotplugged back is as part
> > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > > hotplug state machine while the cpu device associated with that CPU is
> > > not yet ready to be resumed as the device_resume() call happens bit later.
> > > It's not really needed to set the flag is_prepared for cpu devices are
> > > they are mostly pseudo device and hotplug framework deals with state
> > > machine and not managed through the cpu device.
> > >
> > > This often results in annoying warning when resuming:
> > > Enabling non-boot CPUs ...
> > > CPU1: Booted secondary processor
> > > cache: parent cpu1 should not be sleeping
> > > CPU1 is up
> > > CPU2: Booted secondary processor
> > > cache: parent cpu2 should not be sleeping
> > > CPU2 is up
> > > .... and so on.
> > >
> > > Just fix the warning by updating the device state quite early.
> > >
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Reported-by: Jisheng Zhang <jszhang@marvell.com>
> > > Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > > drivers/base/cpu.c | 20 +++++++++++++++++++-
> > > include/linux/cpuhotplug.h | 1 +
> > > 2 files changed, 20 insertions(+), 1 deletion(-)
> > >
> > > Hi Rafael,
> > >
> > > This is getting reported for quite some time. Let me know if you have
> > > better solution to fix this harmless yet annoying warnings during system
> > > resume.
> >
> > I'd rather have a flag in struct dev_pm_info that will cause the message to
> > be suppressed if set.
> >
> > It could be used for other purposes too then in princple (like skipping the
> > creation of empty "power" attr groups in sysfs for devices that don't
> > need them etc.).
> >
> Thanks for the suggestion. I did quick hack and came up with something
> below. I wanted to run through you once before I materialise it into
> a formal patch to check if I understood your suggestion correctly.
> We can move no_pm_required outside dev_pm_info struct and rename with
> any better names.
>
Sorry for the nag, since the title has RFC, thought there are chances of
this getting lost. Let me know if the below idea aligns with your suggestion ?
Regards,
Sudeep
> -->8
>
> diff --git i/drivers/base/cpu.c w/drivers/base/cpu.c
> index eb9443d5bae1..b61f9772ed33 100644
> --- i/drivers/base/cpu.c
> +++ w/drivers/base/cpu.c
> @@ -379,6 +379,7 @@ int register_cpu(struct cpu *cpu, int num)
> cpu->dev.bus->uevent = cpu_uevent;
> #endif
> cpu->dev.groups = common_cpu_attr_groups;
> + cpu->dev.power.no_pm_required = true;
> if (cpu->hotpluggable)
> cpu->dev.groups = hotplugable_cpu_attr_groups;
> error = device_register(&cpu->dev);
> @@ -427,6 +428,7 @@ __cpu_device_create(struct device *parent, void *drvdata,
> dev->parent = parent;
> dev->groups = groups;
> dev->release = device_create_release;
> + dev->power.no_pm_required = true;
> dev_set_drvdata(dev, drvdata);
>
> retval = kobject_set_name_vargs(&dev->kobj, fmt, args);
> diff --git i/drivers/base/power/main.c w/drivers/base/power/main.c
> index 0992e67e862b..ed1b133f73db 100644
> --- i/drivers/base/power/main.c
> +++ w/drivers/base/power/main.c
> @@ -124,6 +124,8 @@ void device_pm_unlock(void)
> */
> void device_pm_add(struct device *dev)
> {
> + if (dev->power.no_pm_required)
> + return;
> pr_debug("PM: Adding info for %s:%s\n",
> dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
> device_pm_check_callbacks(dev);
> @@ -142,6 +144,8 @@ void device_pm_add(struct device *dev)
> */
> void device_pm_remove(struct device *dev)
> {
> + if (dev->power.no_pm_required)
> + return;
> pr_debug("PM: Removing info for %s:%s\n",
> dev->bus ? dev->bus->name : "No Bus", dev_name(dev));
> complete_all(&dev->power.completion);
> diff --git i/drivers/base/power/sysfs.c w/drivers/base/power/sysfs.c
> index d713738ce796..54c1bfec396e 100644
> --- i/drivers/base/power/sysfs.c
> +++ w/drivers/base/power/sysfs.c
> @@ -648,6 +648,9 @@ int dpm_sysfs_add(struct device *dev)
> {
> int rc;
>
> + if (dev->power.no_pm_required)
> + return 0;
> +
> rc = sysfs_create_group(&dev->kobj, &pm_attr_group);
> if (rc)
> return rc;
> diff --git i/include/linux/pm.h w/include/linux/pm.h
> index 0bd9de116826..300ab9f0b858 100644
> --- i/include/linux/pm.h
> +++ w/include/linux/pm.h
> @@ -592,6 +592,7 @@ struct dev_pm_info {
> bool is_suspended:1; /* Ditto */
> bool is_noirq_suspended:1;
> bool is_late_suspended:1;
> + bool no_pm_required:1;
> bool early_init:1; /* Owned by the PM core */
> bool direct_complete:1; /* Owned by the PM core */
> u32 driver_flags;
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-02-04 15:37 ` Sudeep Holla
@ 2019-02-04 15:44 ` Greg Kroah-Hartman
2019-02-04 15:52 ` Sudeep Holla
2019-02-06 10:31 ` Rafael J. Wysocki
1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-04 15:44 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Jisheng Zhang,
Steve Longerbeam, Eugeniu Rosca, Joshua Frkuska
On Mon, Feb 04, 2019 at 03:37:20PM +0000, Sudeep Holla wrote:
> On Thu, Jan 31, 2019 at 04:05:59PM +0000, Sudeep Holla wrote:
> > On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote:
> > > On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> > > > The sysfs for the cpu caches are managed by adding devices with cpu
> > > > as the parent in cpu_device_create() when secondary cpu is brought
> > > > onlin. Generally when the secondary CPUs are hotplugged back is as part
> > > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > > > hotplug state machine while the cpu device associated with that CPU is
> > > > not yet ready to be resumed as the device_resume() call happens bit later.
> > > > It's not really needed to set the flag is_prepared for cpu devices are
> > > > they are mostly pseudo device and hotplug framework deals with state
> > > > machine and not managed through the cpu device.
> > > >
> > > > This often results in annoying warning when resuming:
> > > > Enabling non-boot CPUs ...
> > > > CPU1: Booted secondary processor
> > > > cache: parent cpu1 should not be sleeping
> > > > CPU1 is up
> > > > CPU2: Booted secondary processor
> > > > cache: parent cpu2 should not be sleeping
> > > > CPU2 is up
> > > > .... and so on.
> > > >
> > > > Just fix the warning by updating the device state quite early.
> > > >
> > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > Reported-by: Jisheng Zhang <jszhang@marvell.com>
> > > > Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > > drivers/base/cpu.c | 20 +++++++++++++++++++-
> > > > include/linux/cpuhotplug.h | 1 +
> > > > 2 files changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > Hi Rafael,
> > > >
> > > > This is getting reported for quite some time. Let me know if you have
> > > > better solution to fix this harmless yet annoying warnings during system
> > > > resume.
> > >
> > > I'd rather have a flag in struct dev_pm_info that will cause the message to
> > > be suppressed if set.
> > >
> > > It could be used for other purposes too then in princple (like skipping the
> > > creation of empty "power" attr groups in sysfs for devices that don't
> > > need them etc.).
> > >
> > Thanks for the suggestion. I did quick hack and came up with something
> > below. I wanted to run through you once before I materialise it into
> > a formal patch to check if I understood your suggestion correctly.
> > We can move no_pm_required outside dev_pm_info struct and rename with
> > any better names.
> >
>
> Sorry for the nag, since the title has RFC, thought there are chances of
> this getting lost. Let me know if the below idea aligns with your suggestion ?
Personally, I ignore RFC patches unless I'm accidentally interested in
them, as it shows that the author doesn't feel good enough to propose
them as a real solution :)
But that's just me...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-02-04 15:44 ` Greg Kroah-Hartman
@ 2019-02-04 15:52 ` Sudeep Holla
0 siblings, 0 replies; 21+ messages in thread
From: Sudeep Holla @ 2019-02-04 15:52 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Jisheng Zhang,
Steve Longerbeam, Eugeniu Rosca, Sudeep Holla, Joshua Frkuska
On Mon, Feb 04, 2019 at 04:44:21PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 04, 2019 at 03:37:20PM +0000, Sudeep Holla wrote:
> > On Thu, Jan 31, 2019 at 04:05:59PM +0000, Sudeep Holla wrote:
> > > On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote:
> > > > On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> > > > > The sysfs for the cpu caches are managed by adding devices with cpu
> > > > > as the parent in cpu_device_create() when secondary cpu is brought
> > > > > onlin. Generally when the secondary CPUs are hotplugged back is as part
> > > > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > > > > hotplug state machine while the cpu device associated with that CPU is
> > > > > not yet ready to be resumed as the device_resume() call happens bit later.
> > > > > It's not really needed to set the flag is_prepared for cpu devices are
> > > > > they are mostly pseudo device and hotplug framework deals with state
> > > > > machine and not managed through the cpu device.
> > > > >
> > > > > This often results in annoying warning when resuming:
> > > > > Enabling non-boot CPUs ...
> > > > > CPU1: Booted secondary processor
> > > > > cache: parent cpu1 should not be sleeping
> > > > > CPU1 is up
> > > > > CPU2: Booted secondary processor
> > > > > cache: parent cpu2 should not be sleeping
> > > > > CPU2 is up
> > > > > .... and so on.
> > > > >
> > > > > Just fix the warning by updating the device state quite early.
> > > > >
> > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > Reported-by: Jisheng Zhang <jszhang@marvell.com>
> > > > > Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> > > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > > ---
> > > > > drivers/base/cpu.c | 20 +++++++++++++++++++-
> > > > > include/linux/cpuhotplug.h | 1 +
> > > > > 2 files changed, 20 insertions(+), 1 deletion(-)
> > > > >
> > > > > Hi Rafael,
> > > > >
> > > > > This is getting reported for quite some time. Let me know if you have
> > > > > better solution to fix this harmless yet annoying warnings during system
> > > > > resume.
> > > >
> > > > I'd rather have a flag in struct dev_pm_info that will cause the message to
> > > > be suppressed if set.
> > > >
> > > > It could be used for other purposes too then in princple (like skipping the
> > > > creation of empty "power" attr groups in sysfs for devices that don't
> > > > need them etc.).
> > > >
> > > Thanks for the suggestion. I did quick hack and came up with something
> > > below. I wanted to run through you once before I materialise it into
> > > a formal patch to check if I understood your suggestion correctly.
> > > We can move no_pm_required outside dev_pm_info struct and rename with
> > > any better names.
> > >
> >
> > Sorry for the nag, since the title has RFC, thought there are chances of
> > this getting lost. Let me know if the below idea aligns with your suggestion ?
>
> Personally, I ignore RFC patches unless I'm accidentally interested in
> them, as it shows that the author doesn't feel good enough to propose
> them as a real solution :)
>
I understand. Since Rafael did suggest alternate approach, just thought
of pinging him to check if I understood his idea.
> But that's just me...
>
:)
Regards,
Sudeep
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-02-04 15:37 ` Sudeep Holla
2019-02-04 15:44 ` Greg Kroah-Hartman
@ 2019-02-06 10:31 ` Rafael J. Wysocki
2019-02-06 13:59 ` Sudeep Holla
1 sibling, 1 reply; 21+ messages in thread
From: Rafael J. Wysocki @ 2019-02-06 10:31 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Eugeniu Rosca, Joshua Frkuska, Greg Kroah-Hartman
On Monday, February 4, 2019 4:37:20 PM CET Sudeep Holla wrote:
> On Thu, Jan 31, 2019 at 04:05:59PM +0000, Sudeep Holla wrote:
> > On Thu, Jan 31, 2019 at 12:48:49AM +0100, Rafael J. Wysocki wrote:
> > > On Friday, January 25, 2019 4:09:06 PM CET Sudeep Holla wrote:
> > > > The sysfs for the cpu caches are managed by adding devices with cpu
> > > > as the parent in cpu_device_create() when secondary cpu is brought
> > > > onlin. Generally when the secondary CPUs are hotplugged back is as part
> > > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > > > hotplug state machine while the cpu device associated with that CPU is
> > > > not yet ready to be resumed as the device_resume() call happens bit later.
> > > > It's not really needed to set the flag is_prepared for cpu devices are
> > > > they are mostly pseudo device and hotplug framework deals with state
> > > > machine and not managed through the cpu device.
> > > >
> > > > This often results in annoying warning when resuming:
> > > > Enabling non-boot CPUs ...
> > > > CPU1: Booted secondary processor
> > > > cache: parent cpu1 should not be sleeping
> > > > CPU1 is up
> > > > CPU2: Booted secondary processor
> > > > cache: parent cpu2 should not be sleeping
> > > > CPU2 is up
> > > > .... and so on.
> > > >
> > > > Just fix the warning by updating the device state quite early.
> > > >
> > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > Reported-by: Jisheng Zhang <jszhang@marvell.com>
> > > > Reported-by: Steve Longerbeam <slongerbeam@gmail.com>
> > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > > drivers/base/cpu.c | 20 +++++++++++++++++++-
> > > > include/linux/cpuhotplug.h | 1 +
> > > > 2 files changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > Hi Rafael,
> > > >
> > > > This is getting reported for quite some time. Let me know if you have
> > > > better solution to fix this harmless yet annoying warnings during system
> > > > resume.
> > >
> > > I'd rather have a flag in struct dev_pm_info that will cause the message to
> > > be suppressed if set.
> > >
> > > It could be used for other purposes too then in princple (like skipping the
> > > creation of empty "power" attr groups in sysfs for devices that don't
> > > need them etc.).
> > >
> > Thanks for the suggestion. I did quick hack and came up with something
> > below. I wanted to run through you once before I materialise it into
> > a formal patch to check if I understood your suggestion correctly.
> > We can move no_pm_required outside dev_pm_info struct and rename with
> > any better names.
> >
>
> Sorry for the nag, since the title has RFC, thought there are chances of
> this getting lost. Let me know if the below idea aligns with your suggestion ?
RFC would be fine, but Patchwork doesn't pick up patches posted as replies
in the middle of a thread. :-)
Yes, this is basically what I suggested, please post.
Cheers,
Rafael
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
2019-02-06 10:31 ` Rafael J. Wysocki
@ 2019-02-06 13:59 ` Sudeep Holla
0 siblings, 0 replies; 21+ messages in thread
From: Sudeep Holla @ 2019-02-06 13:59 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, linux-kernel, Jisheng Zhang, Steve Longerbeam,
Eugeniu Rosca, Joshua Frkuska, Greg Kroah-Hartman, Sudeep Holla
On Wed, Feb 06, 2019 at 11:31:04AM +0100, Rafael J. Wysocki wrote:
> On Monday, February 4, 2019 4:37:20 PM CET Sudeep Holla wrote:
> >
> > Sorry for the nag, since the title has RFC, thought there are chances of
> > this getting lost. Let me know if the below idea aligns with your suggestion ?
>
> RFC would be fine, but Patchwork doesn't pick up patches posted as replies
> in the middle of a thread. :-)
>
I completely understand and I had intentions to post it as a patch so
that patchwork sees it. Just wanted to check if I align well with what
you suggested.
> Yes, this is basically what I suggested, please post.
>
Thanks for confirming. I will post the patch shortly.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 21+ messages in thread