RE: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Amitkumar Karwar <akarwar@marvell.com>]

Hi Dmitry,

> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, October 26, 2016 10:06 PM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@vger.kernel.org; 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