Re: [PATCH v2 3/3] wifi: ath12k: Add lock to protect the hardware state

[Kalle Valo <kvalo@kernel.org>]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2024-04-24 at 12:50 +0300, Kalle Valo wrote:
>> Johannes Berg <johannes@sipsolutions.net> writes:
>> 
>> > On Wed, 2024-04-24 at 12:39 +0300, Kalle Valo wrote:
>> > > 
>> > > Exactly. Swithing to use wiphy_lock() definitely one of my longterm
>> > > goals in ath12k. I already started working on switching ath12k to use
>> > > wiphy_lock() but IIRC I got blocked by not being able to call
>> > > wiphy_delayed_work_cancel() without taking wiphy_lock.
>> > 
>> > That's because I didn't want to have an async version, but theoretically
>> > we could have a version of it that just cancels the timer. If you don't
>> > hold the wiphy mutex already you could even wait for it to finish. It
>> > just adds more complexity there, and I was trying to make it all a lot
>> > more obvious :)
>> 
>> Yeah, understandable. I think changing ath12k WMI event handling to use
>> wiphy_work makes sense, running them in tasklet context feels overkill.
>
> Maybe. Note that the wiphy lock _can_ create delays etc. here, if you're
> doing other configuration work at the same time, or similar. Though I
> guess eventually you need locking there anyway, to access the HW. So it
> can be a bit of a trade-off.
>
> I expect this will evolve over time even in mac80211, though so far we
> haven't seen any bad effect from it.

Good point. And now that I think of this more I don't think we can use
wiphy_work with WMI events as we are waiting some events while holding
wiphy_lock, that would end up as deadlock. So most likely we need to use
a normal workqueue with WMI events and take wiphy_lock manually in
certain cases. But I'll need to investigate more and do some
experiments.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches