Re: staging: r8188eu: how to handle nested mutex under spinlock

From: Pavel Skripkin
Date: Sun Apr 03 2022 - 09:06:26 EST

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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature