* Kalle Valo <kvalo@xxxxxxxxxxxxxx> [200623 06:46]: > Tony Lindgren <tony@xxxxxxxxxxx> writes: > > > * Kalle Valo <kvalo@xxxxxxxxxxxxxx> [200622 14:15]: > >> Tony Lindgren <tony@xxxxxxxxxxx> 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
