Date: Wed, 29 Nov 2023 22:24:36 -0800 From: Adrian Chadd <adrian.chadd@gmail.com> To: Farhan Khan <farhan@farhan.codes> Cc: freebsd-wireless <freebsd-wireless@freebsd.org> Subject: Re: Why newstate handler runs IEEE80211_LOCK/UNLOCK? Message-ID: <CAJ-VmonnRP0Gt5DwUw5wtiS3TygdsVS20k6HBHxwTa=d7EP2ow@mail.gmail.com> In-Reply-To: <629e3534-705a-4dcc-ad16-edba170da251@app.fastmail.com> References: <629e3534-705a-4dcc-ad16-edba170da251@app.fastmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000794ee0060b58b5a3 Content-Type: text/plain; charset="UTF-8" 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. 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. I tried cleaning that up years ago but it was .. very tricky to try and change. The wireless drivers would need to make sure they do /no/ blocking activity in the newstate routines, which is .. tricky. -adrian > > Thank you, > -- > Farhan Khan > PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE > > > --000000000000794ee0060b58b5a3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Wed, 29 Nov 2023 at 22:12, Farhan = Khan <farhan@farhan.codes> wrote:<br></div><blockquote class=3D"gmail= _quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204= ,204);padding-left:1ex">Hi all,<br> <br> I'm studying the implementations of net80211 and noticed that in all ne= wstate handlers the code begins by running IEEE80211_UNLOCK(ic) and ends wi= th IEEE80211_LOCK(ic). I was not clear on why this was, I would have expect= ed the opposite order. Also, why not just use the softc device-wide mutex o= ver one for ieee80211com. Overall, I do not seem to understand the intent o= f the unlock and am seeking clarification.<br></blockquote><div><br></div><= div>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=C2=A0held issues (since it's a mutex, not a sleep = lock.)</div><div>The newstate code in net80211 at least (now?) runs in a ta= skqueue, so whenever something changes state, it isn't happening in a r= andom drivers rx/tx/ioctl path. That way newstate transitions are at hopefu= lly=C2=A0serialised and not running in overlapping/concurrent threads.</div= ><div><br></div><div>However, since drivers do a /lot/ of potentially sleep= ing work in the newstate path - think all the sleeping that goes on when th= ings wait for firmware commands to complete - you can't hold a mutex ac= ross those.</div><div><br></div><div>I tried cleaning that up years ago but= it was .. very tricky to try and change. The wireless drivers would need t= o make sure they do /no/ blocking activity in the newstate routines, which = is .. tricky.</div><div><br></div><div><br></div><div><br></div><div>-adria= n</div><div><br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" st= yle=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padd= ing-left:1ex"> <br> Thank you,<br> --<br> Farhan Khan<br> PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE<br> <br> <br> </blockquote></div></div> --000000000000794ee0060b58b5a3--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonnRP0Gt5DwUw5wtiS3TygdsVS20k6HBHxwTa=d7EP2ow>