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

["Fabio M. De Francesco" <fmdefrancesco@gmail.com>]

Hello everyone,

I've noticed a couple of SAC bugs in r8188eu. Recently two of them have been 
addressed by me and one by Michael Straube (sorry if I recall it wrongly).

Prof. Julia Lawall wrote with Others in a paper that "[] Code executing in 
atomic context monopolizes a CPU core, and the progress of other threads that 
need to concurrently access the same resources is delayed. Thus the code 
execution in atomic context should complete as quickly as possible. Sleeping 
in atomic context is forbidden, as it can block a CPU core for a long period 
and may lead to a system hang. [] SAC bugs do not always cause problems in real 
execution, and they are often hard to reproduce at runtime. Recent studies [] 
have shown that SAC bugs have caused serious system hangs at runtime. Thus, 
it is necessary to detect and fix SAC bugs in kernel modules.".

Probably the phrase "SAC bugs do not always cause problems in real execution" 
is the key to understand why very few people seems to care and fix them.

While working on my last patch for this driver, I noticed that a Mutex is 
acquired while holding a Spinlock and while bottom halves are disabled.

In staging/r8188eu/core/rtw_ioctl_set.c, the function rtw_set_802_11_disassociate()
calls spin_lock_bh(). While holding that Spinlock, it calls _rtw_pwr_wakeup()
that, in turn, does a call to msleep().

My first question is whether or not msleep() can be called in atomic context. 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?

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? That lock is used to protect struct "pwrctrl_priv". I didn't
yet provide a patch because I'm pretty sure that I'm missing the good and 
correct reasons that led the author(s) to use a Mutex in that context...

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?

Looking forward to reading what you all think about these two questions of mine.

Regards,

Fabio M. De Francesco