Some functions are known to potentially return -EPROBE_DEFER. In such a
case, it is likely that dev_err_probe() is a better choice than err_err().
dev_err_probe():
- is usually less verbose
- generates smaller .o files
- handles -EPROBE_DEFER so that logs are not spammed
- automatically log the error code in a human readable way
Signed-off-by: Christophe JAILLET <[email protected]>
---
This patch is only a PoC to see if there is some interest in such a new
check.
The hard coded '5 lines of context' has been chosen because a typical
pattern is:
clk = devm_clk_get(dev, "clk_lcd");
if (IS_ERR(clk) {
dev_err(dev, "Error meesage\n");
return PTR_ERR(clk);
}
---
scripts/checkpatch.pl | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2737e4ced574..88365749ed2e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2625,6 +2625,9 @@ sub process {
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
+ my $last_function_that_return_defer = "";
+ my $last_function_that_return_defer_linenr = 0;
+
our @report = ();
our $cnt_lines = 0;
our $cnt_error = 0;
@@ -7459,6 +7462,17 @@ sub process {
WARN("DUPLICATED_SYSCTL_CONST",
"duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr);
}
+
+# check for places where dev_err_probe() would likely be better than dev_err()
+ if ($line =~ /((?:devm_)?clk_get)s*\(/) {
+ $last_function_that_return_defer = $1;
+ $last_function_that_return_defer_linenr = $linenr;
+ }
+ if ($last_function_that_return_defer_linenr >= ($linenr - 5) &&
+ $line =~ /dev_err[^_]/) {
+ WARN("LIKELY_DEV_ERR_PROBE",
+ "dev_err_probe() is likely a better choice than err_err() after a " . $last_function_that_return_defer . "() call\n" . $herecurr);
+ }
}
# If we have no input at all, then there is nothing to report on
--
2.34.1
Le 11/09/2022 à 15:15, Christophe JAILLET a écrit :
> Some functions are known to potentially return -EPROBE_DEFER. In such a
> case, it is likely that dev_err_probe() is a better choice than err_err().
>
> dev_err_probe():
> - is usually less verbose
> - generates smaller .o files
> - handles -EPROBE_DEFER so that logs are not spammed
> - automatically log the error code in a human readable way
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> This patch is only a PoC to see if there is some interest in such a new
> check.
> The hard coded '5 lines of context' has been chosen because a typical
> pattern is:
>
> clk = devm_clk_get(dev, "clk_lcd");
> if (IS_ERR(clk) {
> dev_err(dev, "Error meesage\n");
> return PTR_ERR(clk);
> }
(adding Linus Walleij)
I forgot to say that this patch is a try to address the comment from
Linus Walleij at [1].
It would not help "fixing a gazillion dev_err_probe()", but it could
help not having more to fix later :)
CJ
[1]:
https://lore.kernel.org/all/CACRpkdZEcTD1A3tR=d4fDF89ECMDfchVPW921v6X6ARiPXHEMQ@mail.gmail.com/
> ---
> scripts/checkpatch.pl | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2737e4ced574..88365749ed2e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2625,6 +2625,9 @@ sub process {
> my $last_blank_line = 0;
> my $last_coalesced_string_linenr = -1;
>
> + my $last_function_that_return_defer = "";
> + my $last_function_that_return_defer_linenr = 0;
> +
> our @report = ();
> our $cnt_lines = 0;
> our $cnt_error = 0;
> @@ -7459,6 +7462,17 @@ sub process {
> WARN("DUPLICATED_SYSCTL_CONST",
> "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr);
> }
> +
> +# check for places where dev_err_probe() would likely be better than dev_err()
> + if ($line =~ /((?:devm_)?clk_get)s*\(/) {
> + $last_function_that_return_defer = $1;
> + $last_function_that_return_defer_linenr = $linenr;
> + }
> + if ($last_function_that_return_defer_linenr >= ($linenr - 5) &&
> + $line =~ /dev_err[^_]/) {
> + WARN("LIKELY_DEV_ERR_PROBE",
> + "dev_err_probe() is likely a better choice than err_err() after a " . $last_function_that_return_defer . "() call\n" . $herecurr);
> + }
> }
>
> # If we have no input at all, then there is nothing to report on
On Sun, Sep 11, 2022 at 3:21 PM Christophe JAILLET
<[email protected]> wrote:
> Le 11/09/2022 à 15:15, Christophe JAILLET a écrit :
> > Some functions are known to potentially return -EPROBE_DEFER. In such a
> > case, it is likely that dev_err_probe() is a better choice than err_err().
> >
> > dev_err_probe():
> > - is usually less verbose
> > - generates smaller .o files
> > - handles -EPROBE_DEFER so that logs are not spammed
> > - automatically log the error code in a human readable way
> >
> > Signed-off-by: Christophe JAILLET <[email protected]>
> > ---
> > This patch is only a PoC to see if there is some interest in such a new
> > check.
> > The hard coded '5 lines of context' has been chosen because a typical
> > pattern is:
> >
> > clk = devm_clk_get(dev, "clk_lcd");
> > if (IS_ERR(clk) {
> > dev_err(dev, "Error meesage\n");
> > return PTR_ERR(clk);
> > }
>
> (adding Linus Walleij)
>
>
> I forgot to say that this patch is a try to address the comment from
> Linus Walleij at [1].
>
> It would not help "fixing a gazillion dev_err_probe()", but it could
> help not having more to fix later :)
Needless to say I am a big fan of this patch!
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij