On Fri, 13 Dec 2019 14:05:31 +0100, Kai Vehmanen wrote:
Hey,
On Tue, 10 Dec 2019, Takashi Iwai wrote:
This patch attempts to improve the situation by introducing the standard waitqueue in the RIRB waiter side instead of polling. The
this patch was part of the testing as well, so looks good. One minor nit only:
@@ -216,6 +216,9 @@ void snd_hdac_bus_update_rirb(struct hdac_bus *bus) else if (bus->rirb.cmds[addr]) { bus->rirb.res[addr] = res; bus->rirb.cmds[addr]--;
if (!bus->rirb.cmds[addr] &&waitqueue_active(&bus->rirb_wq))wake_up(&bus->rirb_wq);Checkpath would like to have a comment here:
WARNING: waitqueue_active without comment #77: FILE: sound/hda/hdac_controller.c:220:
waitqueue_active(&bus->rirb_wq))
Yeah, that was known to me, too. Actually it's misleading; what actually matters is the memory barrier or other synchronization there, and this should work as is because of the current code path. (Besides, majority of existing waitqueue_active() have no comments at all :)
And, now I found wa_has_sleeper() as a better replacement of waitqueue_active() that explicitly cares about synchronization. So I'll change with this in a later patch (after unification to hda-core).
thanks,
Takashi