On Fri, 19. Sep 00:50, Ping-Ke Shih wrote:
Ping-Ke Shih pkshih@realtek.com wrote:
Fedor Pchelkin pchelkin@ispras.ru wrote:
That's a good question and it made me rethink the cause of the race scenario. I didn't initially take TX kick off into consideration when it actually matters.
Do it mean that you pictured the racing scenario in commit message by code review instead of a real case you met?
Yes, the underlying issue for this patch was found by code review only. Somehow the negative consequences of the potential race became an "obvious" thing after preparing the first commit, and ignorance of TX kick off influence made the changelog confusing..
The thing is: there might have been another thread initiating TX kick off for the same queue in parallel. But no such thread exists because a taken wiphy lock generally protects from such situations. rtw89_core_txq_schedule() worker looks like a good candidate but it doesn't operate on the needed management queues.
Last night I also thought if another thread works in parallel. Maybe rtw89_ops_tx() could be?
Well, probably it could. I thought rtw89_ops_tx() is wiphy locked, too, but apparently it's not always the case.
Not that it's a relatively easy-to-hit race I'm going to try to reproduce though :)
So I may conclude this patch doesn't fix any real bug though I'd prefer to keep it (with description rewritten of course) because it helps to avoid potential issues in future.
Agree.
Forgot to say. Could you mention this racing scenario was found by core review and your perspective in commit message?
Sure.