On Friday, March 16, 2018 11:22 AM, Kalle Valo wrote: > Daniel Mack <daniel@xxxxxxxxxx> 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. > Well, it's a race which can (theoretically) happen anytime. But sure, I will add another sentence to the commit log and resend. Thanks, Daniel
