How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[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

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

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[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

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

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Sudeep Holla]

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,

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Sudeep Holla]

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,

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Jisheng Zhang]

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,

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Jisheng Zhang]

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,

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Steve Longerbeam]

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
>

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Steve Longerbeam]

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
>

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[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?

Thank you very much,
Eugeniu.

How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[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?

Thank you very much,
Eugeniu.

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Eugeniu Rosca]

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.

Re: How to remove warn msg "cache: parent cpui should not be sleeping" i=1, 2, 3...

[Eugeniu Rosca]

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

[RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[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 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

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[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>

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[Rafael J. Wysocki]

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
> 
> 

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[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;

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[Sudeep Holla]

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;

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[Greg Kroah-Hartman]

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

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[Sudeep Holla]

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

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[Rafael J. Wysocki]

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

Re: [RFC PATCH] drivers core: cpu: add hotplug callback to update cpu_dev state to resumed

[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