mdelay() is not a preferred API to be used to insert delay in the kernel
code unless the context is atomic. Instead, msleep() API can be used.
This patch introduces this warning.
Signed-off-by: Israel Schlesinger <[email protected]>
Signed-off-by: Stepan Moskovchenko <[email protected]>
Signed-off-by: Prakruthi Deepak Heragu <[email protected]>
---
scripts/checkpatch.pl | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a9c0550..14bba3f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5572,6 +5572,12 @@ sub process {
"Comparing get_jiffies_64() is almost always wrong; prefer time_after64, time_before64 and friends\n" . $herecurr);
}
+# check the patch for use of mdelay
+ if ($line =~ /\bmdelay\s*\(/) {
+ WARN("MDELAY",
+ "use of mdelay() found: msleep() is the preferred API.\n" . $herecurr );
+ }
+
# warn about #ifdefs in C files
# if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) {
# print "#ifdef in C files should be avoided\n";
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On Wed, 2018-07-04 at 11:18 -0700, Prakruthi Deepak Heragu wrote:
> mdelay() is not a preferred API to be used to insert delay in the kernel
> code unless the context is atomic. Instead, msleep() API can be used.
> This patch introduces this warning.
[]
> Signed-off-by: Israel Schlesinger <[email protected]>
> Signed-off-by: Stepan Moskovchenko <[email protected]>
> Signed-off-by: Prakruthi Deepak Heragu <[email protected]>
Really? 3 sign-offs for one trivial patch while
getting simple whitespace issues wrong?
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5572,6 +5572,12 @@ sub process {
> "Comparing get_jiffies_64() is almost always wrong; prefer time_after64, time_before64 and friends\n" . $herecurr);
> }
>
> +# check the patch for use of mdelay
> + if ($line =~ /\bmdelay\s*\(/) {
> + WARN("MDELAY",
> + "use of mdelay() found: msleep() is the preferred API.\n" . $herecurr );
No space after $herecurr
> + }
> +
NACK - I think this is unreasonable.
checkpatch is stupid and can only remain that way.
It cannot check for any particular use in atomic
that is appropriate and should not warn with a
false positive when the use is appropriate.
$ git grep -w mdelay | wc -l
2700
How many of those are correct?
If you want a check like this to be useful, write
something for coccinelle or smatch.
Neither Smatch nor Coccinelle do a good job tracking when you're in
atomic context. I've wanted to add this to Smatch but even then it
would be to warn that "We're holding a spinlock so we can't sleep".
It's trickier to say for sure when you're not holding a lock...
regards,
dan carpenter
On Thu, 5 Jul 2018, Dan Carpenter wrote:
> Neither Smatch nor Coccinelle do a good job tracking when you're in
> atomic context. I've wanted to add this to Smatch but even then it
> would be to warn that "We're holding a spinlock so we can't sleep".
> It's trickier to say for sure when you're not holding a lock...
Jia-Ju Bai is working on this. The tool is available on github. It's
still being improved, though, so perhaps it's not yet ready for eg 0-day
inclusion. He can give more details.
julia
On 2018/7/6 13:49, Julia Lawall wrote:
>
> On Thu, 5 Jul 2018, Dan Carpenter wrote:
>
>> Neither Smatch nor Coccinelle do a good job tracking when you're in
>> atomic context. I've wanted to add this to Smatch but even then it
>> would be to warn that "We're holding a spinlock so we can't sleep".
>> It's trickier to say for sure when you're not holding a lock...
> Jia-Ju Bai is working on this. The tool is available on github. It's
> still being improved, though, so perhaps it's not yet ready for eg 0-day
> inclusion. He can give more details.
Thanks for Julia's recommendation :)
I am doing the similar work with Julia, from the beginning of this year.
We develop two new LLVM-based tools to find two problems in the Linux
kernel:
(1) Sleeping in atomic context. The tool is named DSAC.
(2) Using non-sleep function calls in non-atomic context. The tool is
named DCNS.
We handle two common examples of atomic context:
(1) Holding a spinlock.
(2) In an interrupt handler.
DSAC and DCNS can basically work now, and some of the defects found by
them have been confirmed and fixed in the Linux kernel.
But these tools are still being improved.
In fact, I encounter a hard problem when writing the tools, namely how
to accurately and completely handle function pointer calls.
I have handled the function pointer in form of data structure field, but
I do not find a good way to handle the function pointer that is used as
a function argument.
Can someone give me good advice?
We also have made slides introducing DSAC and DCNS tools.
If you are interested in our work, I can send you the slides :)
Best wishes,
Jia-Ju Bai