Hi Dmitry, > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] > Sent: Wednesday, October 26, 2016 10:06 PM > To: Amitkumar Karwar > Cc: Brian Norris; linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant > Sarmukadam > Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' > in shutdown_drv > > Hi Amit, > > On Wed, Oct 26, 2016 at 03:23:08PM +0000, Amitkumar Karwar wrote: > > > > This race won't occur. At this point of time(i.e while calling > mwifiex_shutdown_drv() in deinit), following things are completed. We > don't expect mwifiex_main_process() to be scheduled. > > 1) Connection to peer device is terminated at the beginning of > teardown thread. So we don't receive any Tx data from kernel. > > 2) Last command SHUTDOWN is exchanged with firmware. So there won't be > any activity/interrupt from firmware. > > 3) Interrupts are disabled. > > 4) "adapter->surprise_removed" flag is set. It will skip > mwifiex_main_process() calls. > > > > ----------- > > static void mwifiex_main_work_queue(struct work_struct *work) { > > struct mwifiex_adapter *adapter = > > container_of(work, struct mwifiex_adapter, main_work); > > > > if (adapter->surprise_removed) > > return; > > mwifiex_main_process(adapter); } > > ---------- > > 5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and > destroy workqueue. > > OK, but if interrupts are disabled and you ensure that work is flushed > or completed before you call mwifiex_shutdown_drv() then I do not > understand why you need all of this at all? Why do you need to check > status in mwifiex_shutdown_drv() and why do you want > mwifiex_main_process() to call mwifiex_shutdown_drv() in certain cases? > Can you simply remove all this stuff? > I agree. This code is there for long time. I will prepare a patch for this cleanup work. Regards, Amitkumar
