Thread (8 messages) 8 messages, 2 authors, 2021-10-11

Re: [PATCH v2] staging: r8188eu: Provide a TODO file for this driver

From: Fabio M. De Francesco <hidden>
Date: 2021-10-11 09:53:47
Also in: lkml

On Monday, October 11, 2021 10:21:34 AM CEST Dan Carpenter wrote:
On Sun, Oct 10, 2021 at 11:21:49AM +0200, Fabio M. De Francesco wrote:
quoted
On Saturday, October 9, 2021 6:31:12 PM CEST Fabio M. De Francesco wrote:
quoted
On Thursday, August 26, 2021 3:54:13 PM CEST Dan Carpenter wrote:
quoted
Another thing to fix are some of the sleeping in atomic bugs.

drivers/staging/r8188eu/core/rtw_ap.c:139 update_BCNTIM() warn: 
sleeping
quoted
quoted
quoted
in atomic context
drivers/staging/r8188eu/core/rtw_ap.c:1296 update_bcn_wps_ie() warn: 
sleeping in atomic context

[...]
Hello Dan,

I'd like to address these kind of bugs, but I have a couple of 
questions 
quoted
quoted
about them.

1) You've listed what looks like the output of a compiler or static
analyzer. 
How did you get the warnings you copy-pasted above?

2) I know that both the execution of interrupt handlers (ISRs) as well 
as
quoted
quoted
any 
code blocks that are executed holding spinlocks are "atomic contexts". 
In 
quoted
quoted
these cases, "sleeping" is not allowed (for obvious reasons). Besides 
the
quoted
quoted
two 
mentioned above, are there any further cases of "atomic contexts" in 
the 
quoted
quoted
kernel?
After some research, I've found that Softirqs and Tasklets are also 
executed 
quoted
in "atomic context", as hardware interrupt service routines are.

Furthermore, I've also found a .config option named DEBUG_ATOMIC_SLEEP
that should warn if some code is sleeping in "atomic context". However, 
the 
quoted
documentation of that option does not explain where the output of these 
checks can be read.

I would appreciate any help on this matter.
These are a new Smatch warning.

https://lkml.org/lkml/2021/9/1/950

You would need to rebuild the Smatch database probably around five times
to see the warnings.  It takes a long time to build...  It's probably
not that hard to figure out just from looking at the code without the
generating the warning.
I must confess that, since I started to submit patches to the Kernel this 
year in April and until now, I have not ever used Smatch. I thought that 
building with GCC and setting C=2 and W=1 were enough. Sometimes I've 
also used Coccinelle. That's all.

Now I admit that I was plainly wrong. My _big_ fault, sorry :(

This morning I have taken a quick look at your code at https://github.com/
error27/smatch. Obviously, what I could see is only the overall design and it 
looks quite impressive. I'll start using Smatch soon.
So spin locks can't sleep.  Mutexes can.  There are read/write locks,
built on both mutexes and spin locks.  Rcu_read/write cannot sleep.
Oh, right. RCU were missing from my lists.

Thanks very much for Smatch and for your kind replies.

Regards,

Fabio M. De Francesco

P.S.: Yesterday I read the first one third of a paper co-authored by Julia 
Lawall (Effective Detection of Sleep-in-atomic-context Bugs
in the Linux Kernel - https://doi.org/10.1145/3381990). 

It talks about a practical static approach named DSAC. It looks really 
interesting, but it seems that their tool is not yet available to the public. 

Do you know something about it? I've found an old message by Greg K-H that 
asked for where to find that tool but, as far as I know, he have not yet had 
that information. With this message I've Cc'd Julia in case she has time to 
reply.
regards,
dan carpenter


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help