Re: [RFC] staging: r8188eu: Sleeping in atomic context (SAC) bugs

[Dan Carpenter <dan.carpenter@oracle.com>]

On Mon, Feb 07, 2022 at 01:02:17AM +0100, Fabio M. De Francesco wrote:
> My first question is whether or not msleep() can be called in atomic context.

You are not allowed to call msleep() in atomic context.  The Smatch
check for sleeping in atomic did not look for msleep so I will update
it.

> If
> I understand its semantics and implementation it seems that it should be forbidden.
> What about changing it to mdelay()? Again, it seems that mdelay() spins without 
> sleeping so it should be safe. Isn't it?

mdelay() is does not sleep, but it's not necessarily a good idea to
delay for a long time while holding a spinlock.

> 
> Furthermore, I noticed that _rtw_pwr_wakeup() calls ips_leave(). The latter 
> acquires the "pwrpriv->lock" Mutex. Aren't we forbidden to call Mutexes while 
> holding Spinlocks?

Correct.  You cannot take a mutex while holding a spinlock.

Where is the spinlock in taken in the code you're talking about?  If
it's rtw_set_802_11_disassociate() then that's fine.  The check for
if (check_fwstate(pmlmepriv, _FW_LINKED)) { will prevent ips_leave()
from being called.

> My second question is: should we substitute that Mutex with a Spinlock and use 
> it everywhere else the struct "pwrctrl_priv" must be protected in the driver?

Changing mutexes to spinlocks is tricky.  I can't review your proposed
patch before you send it.

regards,
dan carpenter