Hi Brian/Dmitry, > From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx] > Sent: Tuesday, October 25, 2016 5:26 AM > To: Dmitry Torokhov > Cc: Amitkumar Karwar; linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant > Sarmukadam; briannorris@xxxxxxxxxx > Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' > in shutdown_drv > > Hi, > > On Mon, Oct 24, 2016 at 04:47:20PM -0700, Dmitry Torokhov wrote: > > On Mon, Oct 24, 2016 at 07:51:29PM +0530, Amitkumar Karwar wrote: > > > This variable is guarded by spinlock at all other places. This patch > > > takes care of missing spinlock usage in mwifiex_shutdown_drv(). > > > > > > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> > > > --- > > > drivers/net/wireless/marvell/mwifiex/init.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c > > > b/drivers/net/wireless/marvell/mwifiex/init.c > > > index 82839d9..8e5e424 100644 > > > --- a/drivers/net/wireless/marvell/mwifiex/init.c > > > +++ b/drivers/net/wireless/marvell/mwifiex/init.c > > > @@ -670,11 +670,14 @@ mwifiex_shutdown_drv(struct mwifiex_adapter > > > *adapter) > > > > > > adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING; > > > /* wait for mwifiex_process to complete */ > > > + spin_lock_irqsave(&adapter->main_proc_lock, flags); > > > if (adapter->mwifiex_processing) { > > > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags); > > > mwifiex_dbg(adapter, WARN, > > > "main process is still running\n"); > > > return ret; > > > } > > > + spin_unlock_irqrestore(&adapter->main_proc_lock, flags); > > > > What happens if adapter->mwifiex_processing will become true here? > > That's why I commented: > > "I'm not quite sure why we have this check in the first place; if the > main process is still running, then don't we have bigger problems here > anyway?" If mwifiex_processing is true here, it means main_process is still running. We have set "adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING" in mwifiex_shutdown_drv(). Now we will wait until main_process() gets completed. --------------------- if (mwifiex_shutdown_drv(adapter) == -EINPROGRESS) wait_event_interruptible(adapter->init_wait_q, adapter->init_wait_q_woken); -------------- We have following logic at the end of main_process() ------- exit_main_proc: if (adapter->hw_status == MWIFIEX_HW_STATUS_CLOSING) mwifiex_shutdown_drv(adapter); -------- Regards, Amitkumar Karwar
