[RFC PATCH 0/1] Guiding Use of ssleep over msleep for Whole-Second Delays

[RFC PATCH 0/1] Guiding Use of ssleep over msleep for Whole-Second Delays

[Li Chen]

This change particularly targets replacing msleep() with ssleep() for
delays that are clear multiples of 1000ms, enhancing code semantics and
readability.

I'm contemplating a kernel-wide update to conform to this practice,
pending community feedback. While a sed script
(like '/\bmsleep\s*\(\s*([0-9]+000)\s*\);/{s/\bmsleep\s*\(\s*([0-9]+)000\s*\);/ssleep(\1);/g}')
could automate this, I'm cautious of potential implications and seek
your thoughts on both the patch and the idea of a broad codebase update.

Li Chen (1):
  checkpatch: Add warning for msleep with durations suitable for ssleep

 scripts/checkpatch.pl | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
2.44.0

[RFC PATCH 1/1] checkpatch: Add warning for msleep with durations suitable for ssleep

[Li Chen]

From: Li Chen <chenl311@chinatelecom.cn>

This patch enhances checkpatch.pl by introducing a warning for instances
where msleep() is used with durations that are multiples of 1000
milliseconds. Such durations are more semantically appropriate for
ssleep().

The goal is to encourage developers to use the most fitting sleep function,
improving code readability.

Warn when msleep(x000); can be replaced with ssleep(x);
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 scripts/checkpatch.pl | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..a32df4ead5d4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6599,6 +6599,16 @@ sub process {
 			}
 		}
 
+# warn about msleep() calls with durations that should use ssleep()
+if ($line =~ /\bmsleep\s*\((\d+)\);/) {
+    my $ms_duration = $1;
+    if ($ms_duration >= 1000 && ($ms_duration % 1000) == 0) {
+        my $ss_duration = $ms_duration / 1000;
+        WARN("SSLEEP",
+             "Prefer ssleep($ss_duration) over msleep($ms_duration);\n" . $herecurr);
+    }
+}
+
 # check for comparisons of jiffies
 		if ($line =~ /\bjiffies\s*$Compare|$Compare\s*jiffies\b/) {
 			WARN("JIFFIES_COMPARISON",
-- 
2.44.0

Re: [RFC PATCH 1/1] checkpatch: Add warning for msleep with durations suitable for ssleep

[Joe Perches]

On Tue, 2024-03-05 at 23:47 +0800, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
[]
> Warn when msleep(x000); can be replaced with ssleep(x);

While I don't really see the point as msleep is trivial
to read and ssleep is just msleep * 1000 and there are
already 3:1 more msleep(n*1000) uses than there are ssleep(n)

$ git grep -P '\bmsleep\s*\(\s*\d+000\s*\)' | wc -l
267
$ git grep -P '\bssleep\s*\(\s*\d+\s*\)' | wc -l
89

And about the patch itself:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6599,6 +6599,16 @@ sub process {
>  			}
>  		}
>  
> +# warn about msleep() calls with durations that should use ssleep()
> +if ($line =~ /\bmsleep\s*\((\d+)\);/) {

This indentation is incorrect.
And this would be more sensible using
		if ($line =~ /\bmsleep\s*\(\s*(\d+)000\s*\)/) {

to avoid the extra tests below

> +    my $ms_duration = $1;
> +    if ($ms_duration >= 1000 && ($ms_duration % 1000) == 0) {
> +        my $ss_duration = $ms_duration / 1000;
> +        WARN("SSLEEP",
> +             "Prefer ssleep($ss_duration) over msleep($ms_duration);\n" . $herecurr);

And please add a $fix to this so:

		if ($line =~ /\bmsleep\s*\(\s*(\d+)000\s*\)/) {
			$secs = $1;
			if (WARN("SSLEEP",
				 "Prefer ssleep($secs) over msleep(${secs}000\n") &&
			    $fix) {
				$fixed[$fixlinenr] =~ s/\bmsleep\s*\(\s*${secs}000\s*\)/ssleep($secs)/;
			}