Re: [PATCH] wcn36xx: dequeue all pending indicator messages

[Kalle Valo <kvalo@codeaurora.org>]

Daniel Mack <daniel@zonque.org> writes:

> Hi,
>
> On Friday, March 16, 2018 10:09 AM, Ramon Fried wrote:
>> On 3/16/2018 12:37 AM, Daniel Mack wrote:
>>> In case wcn36xx_smd_rsp_process() is called more than once before
>>> hal_ind_work was dispatched, the messages will end up in hal_ind_queue,
>>> but wcn36xx_ind_smd_work() will only look at the first message in that
>>> list.
>>>
>>> Fix this by dequeing the messages from the list in a loop, and only stop
>>> when it's empty.
>> Interesting. does it solve a specific bug ? can you elaborate ?
>
> I'm poking around in the driver to hopefully find issues that cause
> instability and failures in joining networks, which I am seeing a lot.
> There are a number of bug reports regarding this, for instance
>
>   https://bugs.96boards.org/show_bug.cgi?id=538
>   https://bugs.96boards.org/show_bug.cgi?id=319
>
> I'm following your patches and also started to look into the driver
> myself, and during review, I noticed that list handling issue. I have a
> big fat warning locally that would tell me if the list ever contains
> more than one entry, but that never happens, as the indicator messages
> are way too infrequent to trigger the race. So this isn't a real-world
> issue as far as I can tell, but it is still quite obviously a bug. Hence
> I considered posting a patch.

It's a good idea to mention in the commit log if the fix is for a
theoretical issue and does not necessarily fix anything visible. Helps
to understand the background, prioritise which release the fix should go
etc.

-- 
Kalle Valo