* [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-22 0:14 ` Cody P Schafer
0 siblings, 0 replies; 12+ messages in thread
From: Cody P Schafer @ 2014-02-22 0:14 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Cody P Schafer, Madhavan Srinivasan,
Olof Johansson, Paul Gortmaker, Wang Dongsheng
Cc: linux-kernel, linuxppc-dev, Paul Mackerras
/sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
nothing. Add a pr_warn() to convince any users that they should stop
using it.
The commit message from the removing commit notes that this
functionality should move into the cpuidle driver, essentially by
adjusting target_residency to the specified value. At the moment,
target_residency is not exposed by cpuidle's sysfs, so there isn't a
drop in replacement for this.
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
arch/powerpc/kernel/sysfs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 97e1dc9..84097b4 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -50,6 +50,9 @@ static ssize_t store_smt_snooze_delay(struct device *dev,
if (ret != 1)
return -EINVAL;
+ pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
+ current->comm, task_pid_nr(current), cpu->dev.id);
+
per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
return count;
}
@@ -60,6 +63,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
{
struct cpu *cpu = container_of(dev, struct cpu, dev);
+ pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
+ current->comm, task_pid_nr(current), cpu->dev.id);
+
return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
}
--
1.9.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
2014-02-22 0:14 ` Cody P Schafer
@ 2014-02-25 4:53 ` Madhavan Srinivasan
-1 siblings, 0 replies; 12+ messages in thread
From: Madhavan Srinivasan @ 2014-02-25 4:53 UTC (permalink / raw)
To: Cody P Schafer, Benjamin Herrenschmidt, Olof Johansson,
Paul Gortmaker, Wang Dongsheng
Cc: linuxppc-dev, Paul Mackerras, linux-kernel
On Saturday 22 February 2014 05:44 AM, Cody P Schafer wrote:
> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
> nothing. Add a pr_warn() to convince any users that they should stop
> using it.
>
> The commit message from the removing commit notes that this
> functionality should move into the cpuidle driver, essentially by
Would prefer to cleanup the code since the functionality is moved,
instead of adding to it.
> adjusting target_residency to the specified value. At the moment,
> target_residency is not exposed by cpuidle's sysfs, so there isn't a
> drop in replacement for this.
>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/sysfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 97e1dc9..84097b4 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -50,6 +50,9 @@ static ssize_t store_smt_snooze_delay(struct device *dev,
> if (ret != 1)
> return -EINVAL;
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> return count;
> }
> @@ -60,6 +63,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> {
> struct cpu *cpu = container_of(dev, struct cpu, dev);
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-25 4:53 ` Madhavan Srinivasan
0 siblings, 0 replies; 12+ messages in thread
From: Madhavan Srinivasan @ 2014-02-25 4:53 UTC (permalink / raw)
To: Cody P Schafer, Benjamin Herrenschmidt, Olof Johansson,
Paul Gortmaker, Wang Dongsheng
Cc: Paul Mackerras, linuxppc-dev, linux-kernel
On Saturday 22 February 2014 05:44 AM, Cody P Schafer wrote:
> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
> nothing. Add a pr_warn() to convince any users that they should stop
> using it.
>
> The commit message from the removing commit notes that this
> functionality should move into the cpuidle driver, essentially by
Would prefer to cleanup the code since the functionality is moved,
instead of adding to it.
> adjusting target_residency to the specified value. At the moment,
> target_residency is not exposed by cpuidle's sysfs, so there isn't a
> drop in replacement for this.
>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/sysfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 97e1dc9..84097b4 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -50,6 +50,9 @@ static ssize_t store_smt_snooze_delay(struct device *dev,
> if (ret != 1)
> return -EINVAL;
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> return count;
> }
> @@ -60,6 +63,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> {
> struct cpu *cpu = container_of(dev, struct cpu, dev);
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
2014-02-25 4:53 ` Madhavan Srinivasan
@ 2014-02-25 22:47 ` Cody P Schafer
-1 siblings, 0 replies; 12+ messages in thread
From: Cody P Schafer @ 2014-02-25 22:47 UTC (permalink / raw)
To: Madhavan Srinivasan, Benjamin Herrenschmidt, Olof Johansson,
Paul Gortmaker, Wang Dongsheng
Cc: linuxppc-dev, Paul Mackerras, linux-kernel
On 02/24/2014 08:53 PM, Madhavan Srinivasan wrote:
> On Saturday 22 February 2014 05:44 AM, Cody P Schafer wrote:
>> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
>> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
>> nothing. Add a pr_warn() to convince any users that they should stop
>> using it.
>>
>> The commit message from the removing commit notes that this
>> functionality should move into the cpuidle driver, essentially by
>
> Would prefer to cleanup the code since the functionality is moved,
> instead of adding to it.
We'd still want users of the interface to use an attribute wired up
under the cpuidle/ dir, so a warning (to update their software) is still
needed. As deepthi has noted, cpuidle right now doesn't support changing
this on a per-cpu basis, so a "cleanup" isn't a simple matter.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-25 22:47 ` Cody P Schafer
0 siblings, 0 replies; 12+ messages in thread
From: Cody P Schafer @ 2014-02-25 22:47 UTC (permalink / raw)
To: Madhavan Srinivasan, Benjamin Herrenschmidt, Olof Johansson,
Paul Gortmaker, Wang Dongsheng
Cc: Paul Mackerras, linuxppc-dev, linux-kernel
On 02/24/2014 08:53 PM, Madhavan Srinivasan wrote:
> On Saturday 22 February 2014 05:44 AM, Cody P Schafer wrote:
>> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
>> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
>> nothing. Add a pr_warn() to convince any users that they should stop
>> using it.
>>
>> The commit message from the removing commit notes that this
>> functionality should move into the cpuidle driver, essentially by
>
> Would prefer to cleanup the code since the functionality is moved,
> instead of adding to it.
We'd still want users of the interface to use an attribute wired up
under the cpuidle/ dir, so a warning (to update their software) is still
needed. As deepthi has noted, cpuidle right now doesn't support changing
this on a per-cpu basis, so a "cleanup" isn't a simple matter.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
2014-02-22 0:14 ` Cody P Schafer
@ 2014-02-25 7:59 ` Deepthi Dharwar
-1 siblings, 0 replies; 12+ messages in thread
From: Deepthi Dharwar @ 2014-02-25 7:59 UTC (permalink / raw)
To: Cody P Schafer
Cc: Madhavan Srinivasan, Wang Dongsheng, linux-kernel, Paul Gortmaker,
Paul Mackerras, Olof Johansson, linuxppc-dev
On 02/22/2014 05:44 AM, Cody P Schafer wrote:
> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
> nothing. Add a pr_warn() to convince any users that they should stop
> using it.
>
> The commit message from the removing commit notes that this
> functionality should move into the cpuidle driver, essentially by
> adjusting target_residency to the specified value. At the moment,
> target_residency is not exposed by cpuidle's sysfs, so there isn't a
> drop in replacement for this.
>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
smt-snooze-delay was used to delay an entry into NAP state
or disable NAP state completely. This was before we adopted cpuidle
framework for idle state management on powerpc. This is per-cpu based
tunable, where we could have cores with different target residencies
and idle states.
Now that we have moved towards cpuidle framework, which provides a
better way of idle state management and this framework expects a single
target residency for all the cpus. We can no longer honour
smt-snooze-delay functionality of providing per-cpu target residency.
This was badly broken in the kernel before the patch to clean it up.
By removing this we would honour cpuidle framework through which we
carry out idle state management.
And generic cpuidle framework does not provide the flexibility to change
target residency on the go as there are multiple idle states supported
and trying to change target residency of one state (incorrectly) may
result in undefined behavior.
Also, the second functionality to disable/enable states can be done
using the cpuidle sysfs files. So this is functionality is preserved.
We currently do not use smt-snooze-delay in the kernel.
The sysfs entries needs to be retained until we do a clean up ppc64_cpu
util that uses these entries to determine SMT,
clean up patch for this has already been posted out by Prerna.
Once, we have the ppc64_cpu changes in, we can look to clean up these
parts from the kernel.
Regards,
Deepthi
> ---
> arch/powerpc/kernel/sysfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 97e1dc9..84097b4 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -50,6 +50,9 @@ static ssize_t store_smt_snooze_delay(struct device *dev,
> if (ret != 1)
> return -EINVAL;
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> return count;
> }
> @@ -60,6 +63,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> {
> struct cpu *cpu = container_of(dev, struct cpu, dev);
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-25 7:59 ` Deepthi Dharwar
0 siblings, 0 replies; 12+ messages in thread
From: Deepthi Dharwar @ 2014-02-25 7:59 UTC (permalink / raw)
To: Cody P Schafer
Cc: Benjamin Herrenschmidt, Madhavan Srinivasan, Olof Johansson,
Paul Gortmaker, Wang Dongsheng, Paul Mackerras, linuxppc-dev,
linux-kernel
On 02/22/2014 05:44 AM, Cody P Schafer wrote:
> /sys/devices/system/cpu/cpu*/smt-snooze-delay was converted into a NOP
> in commit 3fa8cad82b94d0bed002571bd246f2299ffc876b, and now does
> nothing. Add a pr_warn() to convince any users that they should stop
> using it.
>
> The commit message from the removing commit notes that this
> functionality should move into the cpuidle driver, essentially by
> adjusting target_residency to the specified value. At the moment,
> target_residency is not exposed by cpuidle's sysfs, so there isn't a
> drop in replacement for this.
>
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
smt-snooze-delay was used to delay an entry into NAP state
or disable NAP state completely. This was before we adopted cpuidle
framework for idle state management on powerpc. This is per-cpu based
tunable, where we could have cores with different target residencies
and idle states.
Now that we have moved towards cpuidle framework, which provides a
better way of idle state management and this framework expects a single
target residency for all the cpus. We can no longer honour
smt-snooze-delay functionality of providing per-cpu target residency.
This was badly broken in the kernel before the patch to clean it up.
By removing this we would honour cpuidle framework through which we
carry out idle state management.
And generic cpuidle framework does not provide the flexibility to change
target residency on the go as there are multiple idle states supported
and trying to change target residency of one state (incorrectly) may
result in undefined behavior.
Also, the second functionality to disable/enable states can be done
using the cpuidle sysfs files. So this is functionality is preserved.
We currently do not use smt-snooze-delay in the kernel.
The sysfs entries needs to be retained until we do a clean up ppc64_cpu
util that uses these entries to determine SMT,
clean up patch for this has already been posted out by Prerna.
Once, we have the ppc64_cpu changes in, we can look to clean up these
parts from the kernel.
Regards,
Deepthi
> ---
> arch/powerpc/kernel/sysfs.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 97e1dc9..84097b4 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -50,6 +50,9 @@ static ssize_t store_smt_snooze_delay(struct device *dev,
> if (ret != 1)
> return -EINVAL;
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
> return count;
> }
> @@ -60,6 +63,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,
> {
> struct cpu *cpu = container_of(dev, struct cpu, dev);
>
> + pr_warn_ratelimited("%s (%d): /sys/devices/system/cpu/cpu%d/smt-snooze-delay is deprecated and is a NOP\n",
> + current->comm, task_pid_nr(current), cpu->dev.id);
> +
> return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
2014-02-25 7:59 ` Deepthi Dharwar
@ 2014-02-25 22:40 ` Benjamin Herrenschmidt
-1 siblings, 0 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-02-25 22:40 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Madhavan Srinivasan, linuxppc-dev, Wang Dongsheng, linux-kernel,
Paul Gortmaker, Paul Mackerras, Olof Johansson, Cody P Schafer
On Tue, 2014-02-25 at 13:29 +0530, Deepthi Dharwar wrote:
> We currently do not use smt-snooze-delay in the kernel.
> The sysfs entries needs to be retained until we do a clean up
> ppc64_cpu
> util that uses these entries to determine SMT,
> clean up patch for this has already been posted out by Prerna.
> Once, we have the ppc64_cpu changes in, we can look to clean up these
> parts from the kernel.
We generally shouldn't change user visible interfaces.
People still have old versions of ppc64_cpu, we must not break them
Cheers,
Ben.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-25 22:40 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-02-25 22:40 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Cody P Schafer, Madhavan Srinivasan, Olof Johansson,
Paul Gortmaker, Wang Dongsheng, Paul Mackerras, linuxppc-dev,
linux-kernel
On Tue, 2014-02-25 at 13:29 +0530, Deepthi Dharwar wrote:
> We currently do not use smt-snooze-delay in the kernel.
> The sysfs entries needs to be retained until we do a clean up
> ppc64_cpu
> util that uses these entries to determine SMT,
> clean up patch for this has already been posted out by Prerna.
> Once, we have the ppc64_cpu changes in, we can look to clean up these
> parts from the kernel.
We generally shouldn't change user visible interfaces.
People still have old versions of ppc64_cpu, we must not break them
Cheers,
Ben.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
2014-02-25 22:40 ` Benjamin Herrenschmidt
@ 2014-02-26 3:45 ` Michael Ellerman
-1 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2014-02-26 3:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Deepthi Dharwar, Madhavan Srinivasan, Cody P Schafer,
Wang Dongsheng, linux-kernel, Paul Gortmaker, Paul Mackerras,
Olof Johansson, linuxppc-dev
On Wed, 2014-02-26 at 09:40 +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2014-02-25 at 13:29 +0530, Deepthi Dharwar wrote:
> > We currently do not use smt-snooze-delay in the kernel.
> > The sysfs entries needs to be retained until we do a clean up
> > ppc64_cpu
> > util that uses these entries to determine SMT,
> > clean up patch for this has already been posted out by Prerna.
> > Once, we have the ppc64_cpu changes in, we can look to clean up these
> > parts from the kernel.
>
> We generally shouldn't change user visible interfaces.
>
> People still have old versions of ppc64_cpu, we must not break them
Yeah we can't remove the file entirely, at least for a few more years.
ppc64_cpu should never have used that file to determine if a cpu existed, but
it did, so we're stuck with it.
What we can do is remove the unused percpu, and just leave the file in sysfs,
and have it print a warning when anyone touches it.
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc: warn users of smt-snooze-delay that the API isn't there anymore
@ 2014-02-26 3:45 ` Michael Ellerman
0 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2014-02-26 3:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Deepthi Dharwar, Madhavan Srinivasan, linuxppc-dev,
Wang Dongsheng, linux-kernel, Paul Gortmaker, Paul Mackerras,
Olof Johansson, Cody P Schafer
On Wed, 2014-02-26 at 09:40 +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2014-02-25 at 13:29 +0530, Deepthi Dharwar wrote:
> > We currently do not use smt-snooze-delay in the kernel.
> > The sysfs entries needs to be retained until we do a clean up
> > ppc64_cpu
> > util that uses these entries to determine SMT,
> > clean up patch for this has already been posted out by Prerna.
> > Once, we have the ppc64_cpu changes in, we can look to clean up these
> > parts from the kernel.
>
> We generally shouldn't change user visible interfaces.
>
> People still have old versions of ppc64_cpu, we must not break them
Yeah we can't remove the file entirely, at least for a few more years.
ppc64_cpu should never have used that file to determine if a cpu existed, but
it did, so we're stuck with it.
What we can do is remove the unused percpu, and just leave the file in sysfs,
and have it print a warning when anyone touches it.
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread