How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...
Subsystems (5)
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?
Thanks in advance,
Jisheng
Quoted 19 lines · Jisheng Zhang
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
Quoted 7 lines
This is because we call
cpu_device_create()when secondary cpu is brought online, thecpu_cachedevice'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
drivers/base/cpu.c +0 -0
@@ -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)
include/linux/cpuhotplug.h +1 -0
@@ -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,
Quoted 25 lines · On Wed, 21 Dec 2016 16:54:22 +0000 Sudeep Holla wrote:
Quoted 24 lines · Jisheng Zhang
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 upThis is because we call
cpu_device_create()when secondary cpu is brought online, thecpu_cachedevice's parent device: cpu device isn't already resumed, all device resume will resume after secondary cores are brought up.
Quoted 9 lines
Quoted 2 lines
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
Quoted 52 lines
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
drivers/base/cpu.c +8
-0@@ -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->hotpluggablefield set to 1 will generate a control file indrivers/base/cpu.c +3
-1@@ -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)include/linux/cpuhotplug.h +1
-0@@ -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,
Hi Sudeep,
Quoted 34 lines · Sudeep Holla
Quoted 19 lines · Jisheng Zhang
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 upQuoted 7 lines
This is because we call
cpu_device_create()when secondary cpu is brought online, thecpu_cachedevice'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. */"
Quoted 3 lines
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
Quoted 54 lines
Regards,
Sudeep-->8
drivers/base/cpu.c +8
-0@@ -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->hotpluggablefield set to 1 will generate a control file indrivers/base/cpu.c +3
-1@@ -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)include/linux/cpuhotplug.h +1
-0@@ -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
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.
Hi Sudeep,
Quoted 5 lines · Eugeniu Rosca
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?
Quoted 2 lines
Thank you very much,
Eugeniu.
RFC PATCH 1/1 drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
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.
--- 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
drivers/base/cpu.c +19 -1
@@ -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)
include/linux/cpuhotplug.h +1 -0
@@ -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
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
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
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
In reply to Sudeep HollaQuoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)
Quoted 5 lines
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.).
Quoted 61 lines
drivers/base/cpu.c +19
-1@@ -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)include/linux/cpuhotplug.h +1
-1@@ -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
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
Quoted 39 lines · Rafael J. Wysocki
Quoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)Quoted 5 lines
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.
Quoted 7 lines
I'd rather have a flag in struct
dev_pm_infothat 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
drivers/base/cpu.c +2 -0
@@ -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);
drivers/base/power/main.c +4 -0
@@ -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);
drivers/base/power/sysfs.c +3 -0
@@ -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;
include/linux/pm.h +1 -0
@@ -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;
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
Quoted 47 lines · Sudeep Holla
Quoted 39 lines · Rafael J. Wysocki
Quoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)Quoted 5 lines
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.
Quoted 7 lines
I'd rather have a flag in struct
dev_pm_infothat 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.).
Quoted 6 lines
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_requiredoutsidedev_pm_infostruct 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
Quoted 70 lines
-->8
drivers/base/cpu.c +2
-0@@ -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);drivers/base/power/main.c +4
-0@@ -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);drivers/base/power/sysfs.c +3
-0@@ -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;include/linux/pm.h +1
-0@@ -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;
Greg Kroah-Hartman
maintainerRe: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
Quoted 55 lines · Sudeep Holla
Quoted 47 lines · Sudeep Holla
Quoted 39 lines · Rafael J. Wysocki
Quoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)Quoted 5 lines
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.
Quoted 7 lines
I'd rather have a flag in struct
dev_pm_infothat 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.).
Quoted 6 lines
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_requiredoutsidedev_pm_infostruct and rename with any better names.
Quoted 2 lines
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
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
Quoted 56 lines · Greg Kroah-Hartman
Quoted 55 lines · Sudeep Holla
Quoted 53 lines · Sudeep Holla
Quoted 39 lines · Rafael J. Wysocki
Quoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)Quoted 5 lines
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.
Quoted 7 lines
I'd rather have a flag in struct
dev_pm_infothat 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_requiredoutsidedev_pm_infostruct and rename with any better names.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 2 lines
But that's just me...
:)
Regards,
Sudeep
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
In reply to Sudeep HollaQuoted 55 lines · CET Sudeep Holla
Quoted 47 lines · Sudeep Holla
Quoted 39 lines · Rafael J. Wysocki
Quoted 32 lines · CET Sudeep Holla
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 callcpu_device_create()from the cpu hotplug state machine while the cpu device associated with that CPU is not yet ready to be resumed as thedevice_resume()call happens bit later. It's not really needed to set the flagis_preparedfor 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" <[email protected]>Reported-by Jisheng Zhang <[email protected]>Reported-by Steve Longerbeam <[email protected]>Reported-by Eugeniu Rosca <[email protected]>Signed-off-by Sudeep Holla <[email protected]>--- drivers/base/cpu.c | 20 +++++++++++++++++++- include/linux/cpuhotplug.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)Quoted 5 lines
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.
Quoted 7 lines
I'd rather have a flag in struct
dev_pm_infothat 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.).
Quoted 6 lines
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_requiredoutsidedev_pm_infostruct and rename with any better names.
Quoted 2 lines
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
Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed
Quoted 8 lines · Rafael J. Wysocki
Quoted 3 lines · CET Sudeep Holla
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.
Quoted 2 lines
Yes, this is basically what I suggested, please post.
Thanks for confirming. I will post the patch shortly.
--
Regards,
Sudeep