From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 8EE33B6F86 for ; Fri, 23 Sep 2011 10:39:28 +1000 (EST) Subject: Re: [PATCH] PSeries: Cancel RTAS event scan before firmware flash From: Benjamin Herrenschmidt To: Ravi K Nittala In-Reply-To: <20110921102825.16444.75131.stgit@localhost6.localdomain6> References: <20110921102825.16444.75131.stgit@localhost6.localdomain6> Content-Type: text/plain; charset="UTF-8" Date: Fri, 23 Sep 2011 10:38:12 +1000 Message-ID: <1316738292.7975.196.camel@pasglop> Mime-Version: 1.0 Cc: linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, 2011-09-21 at 15:59 +0530, Ravi K Nittala wrote: > The RTAS firmware flash update is conducted using an RTAS call that is > serialized by lock_rtas() which uses spin_lock. While the flash is in > progress, rtasd performs scan for any RTAS events that are generated by > the system. rtasd keeps scanning for the RTAS events generated on the > machine. This is performed via workqueue mechanism. The rtas_event_scan() > also uses an RTAS call to scan the events, eventually trying to acquire > the spin_lock before issuing therequest. Better. However: > diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h > index 58625d1..b5cbd9f 100644 > --- a/arch/powerpc/include/asm/rtas.h > +++ b/arch/powerpc/include/asm/rtas.h > @@ -245,6 +245,10 @@ extern int early_init_dt_scan_rtas(unsigned long node, > > extern void pSeries_log_error(char *buf, unsigned int err_type, int fatal); > > +#ifdef CONFIG_PPC_RTAS_DAEMON > +extern bool rtas_cancel_event_scan(void); > +#endif The extern as such doesn't need an ifdef... however, you could avoid this one: .../... > > +#ifdef CONFIG_PPC_RTAS_DAEMON > + /* > + * Just before starting the firmware flash, cancel the event scan work > + * to avoid any soft lockup issues. > + */ > + rtas_cancel_event_scan(); > +#endif > + Here, by having the header contain instead: #ifdef CONFIG_PPC_RTAS_DAEMON extern void rtas_cancel_event_scan(void); #else static inline void rtas_cancel_event_scan(void) { } #endif Also note that I removed the bool, it's not useful since you don't test it anyway. > * NOTE: the "first" block must be under 4GB, so we create > * an entry with no data blocks in the reserved buffer in > 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); > +} Finally, the above is missing an EXPORT_SYMBOL_GPL() since rtas flash can be a module. Cheers, Ben. > static int __init rtas_init(void) > { > struct proc_dir_entry *entry;