Re: [PATCH 1/4] wlcore: Use spin_trylock in wlcore_irq_locked() for running the queue

[Tony Lindgren <tony@atomide.com>]

* Kalle Valo <kvalo@codeaurora.org> [200623 06:46]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Kalle Valo <kvalo@codeaurora.org> [200622 14:15]:
> >> Tony Lindgren <tony@atomide.com> writes:
> >> 
> >> > We need the spinlock to check if we need to run the queue. Let's use
> >> > spin_trylock instead and always run the queue unless we know there's
> >> > nothing to do.
> >> 
> >> Why? What's the problem you are solving here?
> >
> > To simplify the flags and locking use between the threaded irq
> > and tx work.
> >
> > While chasing an occasional hang with an idle wlan doing just a
> > periodic network scans, I noticed we can start simplifying the
> > locking between the threaded irq and tx work for the driver.
> >
> > No luck so far figuring out what the occasional idle wlan hang is,
> > but I suspect we end up somewhere in a deadlock between tx work
> > and the threaded irq.
> >
> > We currently have a collection of flags and locking between the
> > threaded irq and tx work:
> >
> > - wl->flags bitops
> > - wl->mutex
> > - wl->wl_lock spinlock
> >
> > The bitops flags do not need a spinlock around them, and
> > wlcore_irq() already holds the mutex calling wlcore_irq_locked().
> > And we only need the spinlock to see if we need to run the queue
> > or not.
> >
> > So I think eventually we can remove most of the spinlock use in
> > favor of the mutex. I guess I could leave out the trylock changes
> > here if this is too many changes at once.
> >
> > Or do you see some problem in general with this approach?
> 
> My only problem was lack of background information in the commit logs.
> Conditional locking is tricky and I didn't figure out why you are doing
> that and why it's safe to do. So if you could send v2 with the
> information above in the commit log I would be happy.

OK. I'll update the description for the patches and resend.

Thanks,

Tony