mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
To: tglx@linutronix.de, linux-kernel@vger.kernel.org
Cc: "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
Subject: [PATCH RESEND] cpu/hotplug: Wait for cpu_hotplug to be enabled in cpu_up/down
Date: Mon, 3 Feb 2020 07:51:30 +0100	[thread overview]
Message-ID: <d950a169-941e-7caa-608a-df97a127c95d@nokia.com> (raw)

cpu hotplug may be disabled via cpu_hotplug_enable/cpu_hotplug_disable.
When disabled, cpu_down and cpu_up will fail with -EBUSY. Users of the
cpu_up/cpu_down should handle this situation as this is mostly temporal
disablement and exception should be made for EBUSY, assuming that EBUSY
always stands for this situation and is worth repeating execution. This
kind of handling isn't implemented within kernel space users of cpu_up,
cpu_down, while for user space at least implementation within rt-tools
doesn't deal with it.

Instead of creating exceptions for users, this problem should be dealt
at source. One could claim that EBUSY justifies such exception, however
when possible we should create as simple as possible interfaces which
do not fail in case of resource being held.

Heavier user of cpu_hotplug_enable/disable is pci_device_probe yielding
errors on bringing cpu cores up/down if pci devices are getting probed.
Situation in which pci devices are getting probed and having cpus going
down/up is valid situation.

Problem was observed on x86 board by having partitioning of the system
to RT/NRT cpu sets failing (of which part is to bring cpus down/up via
sysfs) if pci devices would be getting probed at the same time. This is
confusing for userspace as dependency to pci devices is not clear.

Fix this behavior by waiting for cpu hotplug to be ready. Return -EBUSY
only after hotplugging was not enabled for about 10 seconds.

Fixes: 1ddd45f8d76f ("PCI: Use cpu_hotplug_disable() instead of get_online_cpus()")
Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
  kernel/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
  1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 9c706af..6ff3c1a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -31,6 +31,7 @@
  #include <linux/relay.h>
  #include <linux/slab.h>
  #include <linux/percpu-rwsem.h>
+#include <linux/wait.h>
  
  #include <trace/events/power.h>
  #define CREATE_TRACE_POINTS
@@ -278,11 +279,22 @@ void cpu_maps_update_done(void)
  }
  
  /*
- * If set, cpu_up and cpu_down will return -EBUSY and do nothing.
+ * If set, cpu_up and cpu_down will retry for cpu_hotplug_retries and
+ * eventually return -EBUSY if unsuccessful.
   * Should always be manipulated under cpu_add_remove_lock
   */
  static int cpu_hotplug_disabled;
  
+/*
+ * waitqueue for waiting on cpu_hotplug_disabled
+ */
+static DECLARE_WAIT_QUEUE_HEAD(wait_cpu_hp_enabled);
+
+/*
+ * Retries for cpu_hotplug to be enabled by cpu_up/cpu_down.
+ */
+static int cpu_hotplug_retries = 10;
+
  #ifdef CONFIG_HOTPLUG_CPU
  
  DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock);
@@ -341,7 +353,7 @@ static void lockdep_release_cpus_lock(void)
  
  /*
   * Wait for currently running CPU hotplug operations to complete (if any) and
- * disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects
+ * briefly disable CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects
   * the 'cpu_hotplug_disabled' flag. The same lock is also acquired by the
   * hotplug path before performing hotplug operations. So acquiring that lock
   * guarantees mutual exclusion from any currently running hotplug operations.
@@ -366,6 +378,7 @@ void cpu_hotplug_enable(void)
  	cpu_maps_update_begin();
  	__cpu_hotplug_enable();
  	cpu_maps_update_done();
+	wake_up(&wait_cpu_hp_enabled);
  }
  EXPORT_SYMBOL_GPL(cpu_hotplug_enable);
  
@@ -1043,11 +1056,21 @@ static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
  
  static int do_cpu_down(unsigned int cpu, enum cpuhp_state target)
  {
-	int err;
+	int err = -EBUSY, retries = cpu_hotplug_retries;
  
-	cpu_maps_update_begin();
-	err = cpu_down_maps_locked(cpu, target);
-	cpu_maps_update_done();
+	while (retries--) {
+		wait_event_timeout(wait_cpu_hp_enabled,
+				!cpu_hotplug_disabled,
+				HZ);
+		cpu_maps_update_begin();
+		if (cpu_hotplug_disabled) {
+			cpu_maps_update_done();
+			continue;
+		}
+		err = _cpu_down(cpu, 0, target);
+		cpu_maps_update_done();
+		break;
+	}
  	return err;
  }
  
@@ -1171,7 +1194,7 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
  
  static int do_cpu_up(unsigned int cpu, enum cpuhp_state target)
  {
-	int err = 0;
+	int err = 0, retries = cpu_hotplug_retries;
  
  	if (!cpu_possible(cpu)) {
  		pr_err("can't online cpu %d because it is not configured as may-hotadd at boot time\n",
@@ -1186,9 +1209,16 @@ static int do_cpu_up(unsigned int cpu, enum cpuhp_state target)
  	if (err)
  		return err;
  
-	cpu_maps_update_begin();
-
-	if (cpu_hotplug_disabled) {
+	while (--retries) {
+		wait_event_timeout(wait_cpu_hp_enabled,
+				   !cpu_hotplug_disabled,
+				   HZ);
+		cpu_maps_update_begin();
+		if (!cpu_hotplug_disabled)
+			break;
+		cpu_maps_update_done();
+	}
+	if (!retries) {
  		err = -EBUSY;
  		goto out;
  	}
-- 
2.10.2


             reply	other threads:[~2020-02-03  6:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-03  6:51 Matija Glavinic Pecotic [this message]
2020-02-03 18:08 ` Thomas Gleixner
2020-02-04  7:43   ` Matija Glavinic Pecotic
2020-02-04 12:30     ` Thomas Gleixner
2020-02-05 12:49       ` Matija Glavinic Pecotic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d950a169-941e-7caa-608a-df97a127c95d@nokia.com \
    --to=matija.glavinic-pecotic.ext@nokia.com \
    --cc=alexander.sverdlin@nokia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®