Messages in this thread |  | | Date | Sun, 3 Apr 2022 16:02:16 +0300 | Subject | Re: staging: r8188eu: how to handle nested mutex under spinlock | From | Pavel Skripkin <> |
| |
Hi Fabio,
On 4/3/22 15:55, Fabio M. De Francesco wrote: > On domenica 3 aprile 2022 14:45:49 CEST Pavel Skripkin wrote: >> Hi Fabio, >> >> On 4/3/22 15:37, Fabio M. De Francesco wrote: >> >> > >> >> > drivers/staging/r8188eu/core/rtw_pwrctrl.c:379 >> >> > >> >> > if (pwrpriv->ps_processing) { >> >> > while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000) >> >> > msleep(10); >> >> > } >> >> > >> >> >> >> Hm, just wondering, shouldn't we annotate load from >> >> pwrpriv->ps_processing with READ_ONCE() inside while loop? >> >> IIUC compiler might want to cache first load into register and we will >> >> stuck here forever. >> > >> > You're right. This can be cached. In situations like these one should use >> > barriers or other API that use barriers implicitly (completions, for example). >> > >> >> Not sure about completions, since they may sleep. > > No completions in this special context. They for _sure_ might sleep. I was > talking about general cases when you are in a loop and wait for status change. > >> >> Also, don't think that barriers are needed here, since this code just >> waiting for observing value 1. Might be barrier will slightly speed up >> waiting thread, but will also slow down other thread > > Here, I cannot help with a 100% good answer. Maybe Greg wants to say something > about it? >
IMO, the best answer is just remove this loop, since it does nothing. Or redesign it to be more sane
It waits for ps_processing to become 0 for 3000 ms, but if 3000 ms expires... execution goes forward like as ps_processing was 0 from the beginning
Maybe it's something hw related, like wait for 3000 ms and all will be ok. Can't say...
With regards, Pavel Skripkin [unhandled content-type:application/pgp-signature] |  |