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>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] 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 > > > [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 29 Nov 2023 at 22:12, Farhan Khan <farhan@farhan.codes> wrote:<br></div><blockquote class="gmail_quote" style="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 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.<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 held issues (since it's a mutex, not a sleep lock.)</div><div>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.</div><div><br></div><div>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.</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 to 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>-adrian</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-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>home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonnRP0Gt5DwUw5wtiS3TygdsVS20k6HBHxwTa=d7EP2ow>