Re: [PATCH] powerpc: Warn about use of smt_snooze_delay

[Christophe Leroy]

Le 25/06/2020 à 12:03, Joel Stanley a écrit :

It's not done anything for a long time. Save the percpu variable, and
emit a warning to remind users to not expect it to do anything.

Why not just drop the file entirely  if it is useless ?

Christophe

Signed-off-by: Joel Stanley <j...@jms.id.au>
---
  arch/powerpc/kernel/sysfs.c | 41 +++++++++++++------------------------
  1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 571b3259697e..530ae92bc46d 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -32,29 +32,25 @@

static DEFINE_PER_CPU(struct cpu, cpu_devices); -/*

- * SMT snooze delay stuff, 64-bit only for now
- */
-
  #ifdef CONFIG_PPC64

-/* Time in microseconds we delay before sleeping in the idle loop */

-static DEFINE_PER_CPU(long, smt_snooze_delay) = { 100 };
+/*
+ * Snooze delay has not been hooked up since 3fa8cad82b94 
("powerpc/pseries/cpuidle:
+ * smt-snooze-delay cleanup.") and has been broken even longer. As was 
foretold in
+ * 2014:
+ *
+ *  "ppc64_util currently utilises it. Once we fix ppc64_util, propose to clean
+ *  up the kernel code."
+ *
+ * At some point in the future this code should be removed.
+ */

static ssize_t store_smt_snooze_delay(struct device *dev,

                                      struct device_attribute *attr,
                                      const char *buf,
                                      size_t count)
  {
-       struct cpu *cpu = container_of(dev, struct cpu, dev);
-       ssize_t ret;
-       long snooze;
-
-       ret = sscanf(buf, "%ld", &snooze);
-       if (ret != 1)
-               return -EINVAL;
-
-       per_cpu(smt_snooze_delay, cpu->dev.id) = snooze;
+       WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");
        return count;
  }

@@ -62,9 +58,9 @@ static ssize_t show_smt_snooze_delay(struct device *dev,

                                     struct device_attribute *attr,
                                     char *buf)
  {
-       struct cpu *cpu = container_of(dev, struct cpu, dev);
+       WARN_ON_ONCE("smt_snooze_delay sysfs file has no effect\n");

- return sprintf(buf, "%ld\n", per_cpu(smt_snooze_delay, cpu->dev.id));

+       return sprintf(buf, "100\n");
  }

static DEVICE_ATTR(smt_snooze_delay, 0644, show_smt_snooze_delay,

@@ -72,16 +68,7 @@ static DEVICE_ATTR(smt_snooze_delay, 0644, 
show_smt_snooze_delay,

static int __init setup_smt_snooze_delay(char *str)

  {
-       unsigned int cpu;
-       long snooze;
-
-       if (!cpu_has_feature(CPU_FTR_SMT))
-               return 1;
-
-       snooze = simple_strtol(str, NULL, 10);
-       for_each_possible_cpu(cpu)
-               per_cpu(smt_snooze_delay, cpu) = snooze;
-
+       WARN_ON_ONCE("smt-snooze-delay command line option has no effect\n");
        return 1;
  }
  __setup("smt-snooze-delay=", setup_smt_snooze_delay);