linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Ravi K. Nittala" <ravi.nittala@in.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: antonb@au1.ibm.com, subrata.modak@in.ibm.com, mikey@neuling.org,
	sbest@us.ibm.com, suzuki@in.ibm.com, ranittal@linux.vnet.ibm.com,
	divya.vikas@in.ibm.com
Subject: [PATCH] PSeries: Cancel RTAS event scan before firmware flash
Date: Wed, 27 Jul 2011 17:39:20 +0530	[thread overview]
Message-ID: <20110727120801.10429.7276.stgit@localhost6.localdomain6> (raw)

The firmware flash update is conducted using an RTAS call, that is serialized
by lock_rtas() which uses spin_lock. rtasd keeps scanning for the RTAS events
generated on the machine. This is performed via a delayed workqueue, invoking
an RTAS call to scan the events.

The flash update takes a while to complete and during this time, any other
RTAS call has to wait. In this case, rtas_event_scan() waits for a long time
on the spin_lock resulting in a soft lockup.

Approaches to fix the issue :

Approach 1: Stop all the other CPUs before we start flashing the firmware.

Before the rtas firmware update starts, all other CPUs should be stopped.
Which means no other CPU should be in lock_rtas(). We do not want other CPUs
execute while FW update is in progress and the system will be rebooted anyway
after the update.

--- arch/powerpc/kernel/setup-common.c.orig    2011-07-01 22:41:12.952507971 -0400
+++ arch/powerpc/kernel/setup-common.c    2011-07-01 22:48:31.182507915 -0400
@@ -109,11 +109,12 @@ void machine_shutdown(void)
  void machine_restart(char *cmd)
  {
      machine_shutdown();
-    if (ppc_md.restart)
-        ppc_md.restart(cmd);
  #ifdef CONFIG_SMP
-    smp_send_stop();
+        smp_send_stop();
  #endif
+    if (ppc_md.restart)
+        ppc_md.restart(cmd);
+
      printk(KERN_EMERG "System Halted, OK to turn off power\n");
      local_irq_disable();
      while (1) ;

Problems with this approach:
Stopping the CPUs suddenly may cause other serious problems depending on what
was running on them. Hence, this approach cannot be considered.


Approach 2: Cancel the rtas_scan_event work before starting the firmware flash.

Just before the flash update is performed, the queued rtas_event_scan() work
item is cancelled from the work queue so that there is no other RTAS call
issued while the flash is in progress. After the flash completes, the system
reboots and the rtas_event_scan() is rescheduled.

Approach 2 looks to be a better solution than Approach 1. Kindly let us know
your thoughts. Patch attached.


Signed-off-by: Suzuki Poulose <suzuki@in.ibm.com>
Signed-off-by: Ravi Nittala <ravi.nittala@in.ibm.com>


---
 arch/powerpc/include/asm/rtas.h  |    2 ++
 arch/powerpc/kernel/rtas_flash.c |    6 ++++++
 arch/powerpc/kernel/rtasd.c      |    6 ++++++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 58625d1..3f26f87 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -245,6 +245,8 @@ extern int early_init_dt_scan_rtas(unsigned long node,
 
 extern void pSeries_log_error(char *buf, unsigned int err_type, int fatal);
 
+extern bool rtas_cancel_event_scan(void);
+
 /* Error types logged.  */
 #define ERR_FLAG_ALREADY_LOGGED	0x0
 #define ERR_FLAG_BOOT		0x1 	/* log was pulled from NVRAM on boot */
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index e037c74..4174b4b 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -568,6 +568,12 @@ static void rtas_flash_firmware(int reboot_type)
 	}
 
 	/*
+	 * Just before starting the firmware flash, cancel the event scan work
+	 * to avoid any soft lockup issues.
+	 */
+	rtas_cancel_event_scan();
+
+	/*
 	 * NOTE: the "first" block must be under 4GB, so we create
 	 * an entry with no data blocks in the reserved buffer in
 	 * the kernel data segment.
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 481ef06..e8f03fa 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -472,6 +472,12 @@ static void start_event_scan(void)
 				 &event_scan_work, event_scan_delay);
 }
 
+/* Cancel the rtas event scan work */
+bool rtas_cancel_event_scan(void)
+{
+	return cancel_delayed_work_sync(&event_scan_work);
+}
+
 static int __init rtas_init(void)
 {
 	struct proc_dir_entry *entry;

             reply	other threads:[~2011-07-27 12:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-27 12:09 Ravi K. Nittala [this message]
2011-08-08  6:46 ` [PATCH] PSeries: Cancel RTAS event scan before firmware flash Suzuki Poulose
2011-08-30  6:03 ` Benjamin Herrenschmidt
2011-08-30  6:17   ` Suzuki Poulose
2011-08-30  6:19     ` Benjamin Herrenschmidt
2011-08-30  6:21       ` Benjamin Herrenschmidt
2011-08-30  6:57         ` Suzuki Poulose
  -- strict thread matches above, loose matches on Subject: below --
2011-09-21 10:29 Ravi K Nittala
2011-09-23  0:38 ` Benjamin Herrenschmidt
2011-10-04  7:49 Ravi K Nittala
2011-10-04  7:54 ` Subrata Modak

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=20110727120801.10429.7276.stgit@localhost6.localdomain6 \
    --to=ravi.nittala@in.ibm.com \
    --cc=antonb@au1.ibm.com \
    --cc=divya.vikas@in.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mikey@neuling.org \
    --cc=ranittal@linux.vnet.ibm.com \
    --cc=sbest@us.ibm.com \
    --cc=subrata.modak@in.ibm.com \
    --cc=suzuki@in.ibm.com \
    /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;
as well as URLs for NNTP newsgroup(s).