Re: Why newstate handler runs IEEE80211_LOCK/UNLOCK?

From: Farhan Khan <farhan_at_farhan.codes>
Date: Thu, 30 Nov 2023 16:09:53 UTC
On Thu, Nov 30, 2023, at 1:24 AM, Adrian Chadd wrote:
> On Wed, 29 Nov 2023 at 22:12, Farhan Khan <farhan@farhan.codes> wrote:
>> Hi all,
>> 
>> I'm studying the implementations of net80211 and noticed that in all newstate handlers the code begins by running IEEE80211_UNLOCK(ic) and ends with IEEE80211_LOCK(ic). I was not clear on why this was, I would have expected the opposite order. Also, why not just use the softc device-wide mutex over one for ieee80211com. Overall, I do not seem to understand the intent of the unlock and am seeking clarification.
>
> That part of the net80211 locking handling is ... unfortunately unfun. 
> Without doing that, there'd be lots of lock order inversion issues and 
> sleeping whilst lock held issues (since it's a mutex, not a sleep lock.)
> The newstate code in net80211 at least (now?) runs in a taskqueue, so 
> whenever something changes state, it isn't happening in a random 
> drivers rx/tx/ioctl path. That way newstate transitions are at 
> hopefully serialised and not running in overlapping/concurrent threads.

I'm still a little unclear here. Why does it inverted UNLOCK first? Wouldn't that mean the state /can/ change until still be a LOCK first? And, why not just do a softc-wide lock, why IEEE80211's lock function? But then there is also a driver softc lock, which confuses me. I'm also not understanding the double lock mechanism.

>
> However, since drivers do a /lot/ of potentially sleeping work in the 
> newstate path - think all the sleeping that goes on when things wait 
> for firmware commands to complete - you can't hold a mutex across those.

This seems relevant but I did not understand. :/

-- 
Farhan Khan
PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE