* [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
@ 2020-11-21 8:09 Dwaipayan Ray
2020-11-21 9:45 ` Lukas Bulwahn
0 siblings, 1 reply; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-21 8:09 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: dwaipayanray1, linux-kernel-mentees
Provide fix option to INCLUDE_LINUX check to replace asm
includes.
Macros of type:
#include <asm/percpu.h>
are corrected to:
#include <linux/percpu.h>
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0da6422cd0fd..f852f130e065 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5468,8 +5468,11 @@ sub process {
CHK("ARCH_INCLUDE_LINUX",
"Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
} else {
- WARN("INCLUDE_LINUX",
- "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
+ if (WARN("INCLUDE_LINUX",
+ "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\<asm\/$file\>/\<linux\/$file\>/;
+ }
}
}
}
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 8:09 [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX Dwaipayan Ray
@ 2020-11-21 9:45 ` Lukas Bulwahn
2020-11-21 11:55 ` Dwaipayan Ray
0 siblings, 1 reply; 8+ messages in thread
From: Lukas Bulwahn @ 2020-11-21 9:45 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees
On Sat, Nov 21, 2020 at 9:09 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>
> Provide fix option to INCLUDE_LINUX check to replace asm
> includes.
>
> Macros of type:
> #include <asm/percpu.h>
>
> are corrected to:
> #include <linux/percpu.h>
>
The patch itself looks okay; but should this fix option not be limited
to lines where content is added?
Further we need to investigate:
- is this still a valid rule?
- is this rule and the rationale documented somewhere?
- how many rule violations currently exist in the kernel repository?
- are there known exceptions for some files in some directories?
Lukas
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
> scripts/checkpatch.pl | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 0da6422cd0fd..f852f130e065 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5468,8 +5468,11 @@ sub process {
> CHK("ARCH_INCLUDE_LINUX",
> "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> } else {
> - WARN("INCLUDE_LINUX",
> - "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> + if (WARN("INCLUDE_LINUX",
> + "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\<asm\/$file\>/\<linux\/$file\>/;
> + }
> }
> }
> }
> --
> 2.27.0
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 9:45 ` Lukas Bulwahn
@ 2020-11-21 11:55 ` Dwaipayan Ray
2020-11-21 13:12 ` Lukas Bulwahn
0 siblings, 1 reply; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-21 11:55 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On Sat, Nov 21, 2020 at 3:15 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>
> On Sat, Nov 21, 2020 at 9:09 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> >
> > Provide fix option to INCLUDE_LINUX check to replace asm
> > includes.
> >
> > Macros of type:
> > #include <asm/percpu.h>
> >
> > are corrected to:
> > #include <linux/percpu.h>
> >
>
> The patch itself looks okay; but should this fix option not be limited
> to lines where content is added?
>
Hi,
I think the warning itself is emitted for lines which are added.
So I think the fix option even now will change those added lines only.
> Further we need to investigate:
> - is this still a valid rule?
> - is this rule and the rationale documented somewhere?
I think the rule is valid. I was going through the documentation files
and in several examples I saw that the linux headers are preferred.
Like in Documentation/core-api/local_ops.rst:
91:
#include <linux/percpu.h>
#include <asm/local.h>
In Documentation/trace/kprobes.rst:
362:
#include <linux/kprobes.h>
int register_kprobe(struct kprobe *kp);
400:
#include <linux/kprobes.h>
#include <linux/ptrace.h>
int pre_handler(struct kprobe *p, struct pt_regs *regs);
410:
#include <linux/kprobes.h>
#include <linux/ptrace.h>
void post_handler(struct kprobe *p, struct pt_regs *regs,
unsigned long flags);
and so on...
I didn't find any explicit mention about absolutely preffering
linux headers over asm headers whenever it exists.
> - how many rule violations currently exist in the kernel repository?
> - are there known exceptions for some files in some directories?
There are exceptions to this.
Like in include/linux/percpu.h,
#include <asm/percpu.h> needs to be included. as linux/percpu wraps
over asm/percpu. So these kinds are exempted from the check and
checkpatch doesn't report any warning.
I did find some violations like in arch/csky/kernel/probes/decode-insn.h:
CHECK: Consider using #include <linux/kprobes.h> instead of <asm/kprobes.h>
#7: FILE: arch/csky/kernel/probes/decode-insn.h:7:
+#include <asm/kprobes.h>
I found one commit which fixed one such case in the kernel:
commit ca5999fde0a17 ("mm: introduce include/linux/pgtable.h")
It says "The include/linux/pgtable.h is going to be the home of
generic page table
manipulation functions."
So I think it means that future additings should use <linux/pgtable.h>
instead of
<asm/pgtable.h>
I think the ARCH_INCLUDE_LINUX also deserves a --fix option similarly.
What do you think? Should I continue with this?
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 11:55 ` Dwaipayan Ray
@ 2020-11-21 13:12 ` Lukas Bulwahn
2020-11-21 16:16 ` Dwaipayan Ray
0 siblings, 1 reply; 8+ messages in thread
From: Lukas Bulwahn @ 2020-11-21 13:12 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees
On Sat, Nov 21, 2020 at 12:56 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>
> On Sat, Nov 21, 2020 at 3:15 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> >
> > On Sat, Nov 21, 2020 at 9:09 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> > >
> > > Provide fix option to INCLUDE_LINUX check to replace asm
> > > includes.
> > >
> > > Macros of type:
> > > #include <asm/percpu.h>
> > >
> > > are corrected to:
> > > #include <linux/percpu.h>
> > >
> >
> > The patch itself looks okay; but should this fix option not be limited
> > to lines where content is added?
> >
>
> Hi,
> I think the warning itself is emitted for lines which are added.
> So I think the fix option even now will change those added lines only.
>
> > Further we need to investigate:
> > - is this still a valid rule?
> > - is this rule and the rationale documented somewhere?
>
> I think the rule is valid. I was going through the documentation files
> and in several examples I saw that the linux headers are preferred.
>
> Like in Documentation/core-api/local_ops.rst:
>
> 91:
> #include <linux/percpu.h>
> #include <asm/local.h>
>
> In Documentation/trace/kprobes.rst:
>
> 362:
> #include <linux/kprobes.h>
> int register_kprobe(struct kprobe *kp);
>
> 400:
> #include <linux/kprobes.h>
> #include <linux/ptrace.h>
> int pre_handler(struct kprobe *p, struct pt_regs *regs);
>
> 410:
> #include <linux/kprobes.h>
> #include <linux/ptrace.h>
> void post_handler(struct kprobe *p, struct pt_regs *regs,
> unsigned long flags);
>
> and so on...
>
> I didn't find any explicit mention about absolutely preffering
> linux headers over asm headers whenever it exists.
>
> > - how many rule violations currently exist in the kernel repository?
> > - are there known exceptions for some files in some directories?
>
> There are exceptions to this.
> Like in include/linux/percpu.h,
> #include <asm/percpu.h> needs to be included. as linux/percpu wraps
> over asm/percpu. So these kinds are exempted from the check and
> checkpatch doesn't report any warning.
>
> I did find some violations like in arch/csky/kernel/probes/decode-insn.h:
> CHECK: Consider using #include <linux/kprobes.h> instead of <asm/kprobes.h>
> #7: FILE: arch/csky/kernel/probes/decode-insn.h:7:
> +#include <asm/kprobes.h>
>
> I found one commit which fixed one such case in the kernel:
> commit ca5999fde0a17 ("mm: introduce include/linux/pgtable.h")
> It says "The include/linux/pgtable.h is going to be the home of
> generic page table
> manipulation functions."
>
> So I think it means that future additings should use <linux/pgtable.h>
> instead of
> <asm/pgtable.h>
>
> I think the ARCH_INCLUDE_LINUX also deserves a --fix option similarly.
>
> What do you think? Should I continue with this?
>
Yes, that is good. Thanks for the investigation.
You can consider to:
- also fix the cases where this rule is violated and send patches to
the maintainers.
- add some documentation on this rule.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 13:12 ` Lukas Bulwahn
@ 2020-11-21 16:16 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-21 16:16 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On Sat, Nov 21, 2020 at 6:43 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>
> On Sat, Nov 21, 2020 at 12:56 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> >
> > On Sat, Nov 21, 2020 at 3:15 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> > >
> > > On Sat, Nov 21, 2020 at 9:09 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> > > >
> > > > Provide fix option to INCLUDE_LINUX check to replace asm
> > > > includes.
> > > >
> > > > Macros of type:
> > > > #include <asm/percpu.h>
> > > >
> > > > are corrected to:
> > > > #include <linux/percpu.h>
> > > >
> > >
> > > The patch itself looks okay; but should this fix option not be limited
> > > to lines where content is added?
> > >
> >
> > Hi,
> > I think the warning itself is emitted for lines which are added.
> > So I think the fix option even now will change those added lines only.
> >
> > > Further we need to investigate:
> > > - is this still a valid rule?
> > > - is this rule and the rationale documented somewhere?
> >
> > I think the rule is valid. I was going through the documentation files
> > and in several examples I saw that the linux headers are preferred.
> >
> > Like in Documentation/core-api/local_ops.rst:
> >
> > 91:
> > #include <linux/percpu.h>
> > #include <asm/local.h>
> >
> > In Documentation/trace/kprobes.rst:
> >
> > 362:
> > #include <linux/kprobes.h>
> > int register_kprobe(struct kprobe *kp);
> >
> > 400:
> > #include <linux/kprobes.h>
> > #include <linux/ptrace.h>
> > int pre_handler(struct kprobe *p, struct pt_regs *regs);
> >
> > 410:
> > #include <linux/kprobes.h>
> > #include <linux/ptrace.h>
> > void post_handler(struct kprobe *p, struct pt_regs *regs,
> > unsigned long flags);
> >
> > and so on...
> >
> > I didn't find any explicit mention about absolutely preffering
> > linux headers over asm headers whenever it exists.
> >
> > > - how many rule violations currently exist in the kernel repository?
> > > - are there known exceptions for some files in some directories?
> >
> > There are exceptions to this.
> > Like in include/linux/percpu.h,
> > #include <asm/percpu.h> needs to be included. as linux/percpu wraps
> > over asm/percpu. So these kinds are exempted from the check and
> > checkpatch doesn't report any warning.
> >
> > I did find some violations like in arch/csky/kernel/probes/decode-insn.h:
> > CHECK: Consider using #include <linux/kprobes.h> instead of <asm/kprobes.h>
> > #7: FILE: arch/csky/kernel/probes/decode-insn.h:7:
> > +#include <asm/kprobes.h>
> >
> > I found one commit which fixed one such case in the kernel:
> > commit ca5999fde0a17 ("mm: introduce include/linux/pgtable.h")
> > It says "The include/linux/pgtable.h is going to be the home of
> > generic page table
> > manipulation functions."
> >
> > So I think it means that future additings should use <linux/pgtable.h>
> > instead of
> > <asm/pgtable.h>
> >
> > I think the ARCH_INCLUDE_LINUX also deserves a --fix option similarly.
> >
> > What do you think? Should I continue with this?
> >
>
> Yes, that is good. Thanks for the investigation.
>
> You can consider to:
>
> - also fix the cases where this rule is violated and send patches to
> the maintainers.
> - add some documentation on this rule.
>
Okay sure I will try doing that.
I am just sending this patch unchanged to Joe and LKML to have their
review. If it is ok, I will get on with the next task.
Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 20:05 ` Joe Perches
@ 2020-11-21 20:31 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-21 20:31 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel
On Sun, Nov 22, 2020 at 1:35 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-11-21 at 21:47 +0530, Dwaipayan Ray wrote:
> > Provide fix option to INCLUDE_LINUX check to replace asm
> > includes.
> >
> > Macros of type:
> > #include <asm/percpu.h>
> >
> > are corrected to:
> > #include <linux/percpu.h>
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -5468,8 +5468,11 @@ sub process {
> > CHK("ARCH_INCLUDE_LINUX",
> > "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> > } else {
> > - WARN("INCLUDE_LINUX",
> > - "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> > + if (WARN("INCLUDE_LINUX",
> > + "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr) &&
> > + $fix) {
> > + $fixed[$fixlinenr] =~ s/\<asm\/$file\>/\<linux\/$file\>/;
>
> $file can include a slash.
>
> e.g.: arch/arm/kernel/atags_parse.c:#include <asm/mach/arch.h>
>
> Probably simpler to use /Q /E quoting.
>
>
Thanks. I will do that.
Regards,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
2020-11-21 16:17 Dwaipayan Ray
@ 2020-11-21 20:05 ` Joe Perches
2020-11-21 20:31 ` Dwaipayan Ray
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2020-11-21 20:05 UTC (permalink / raw)
To: Dwaipayan Ray; +Cc: linux-kernel-mentees, linux-kernel
On Sat, 2020-11-21 at 21:47 +0530, Dwaipayan Ray wrote:
> Provide fix option to INCLUDE_LINUX check to replace asm
> includes.
>
> Macros of type:
> #include <asm/percpu.h>
>
> are corrected to:
> #include <linux/percpu.h>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5468,8 +5468,11 @@ sub process {
> CHK("ARCH_INCLUDE_LINUX",
> "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> } else {
> - WARN("INCLUDE_LINUX",
> - "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
> + if (WARN("INCLUDE_LINUX",
> + "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\<asm\/$file\>/\<linux\/$file\>/;
$file can include a slash.
e.g.: arch/arm/kernel/atags_parse.c:#include <asm/mach/arch.h>
Probably simpler to use /Q /E quoting.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
@ 2020-11-21 16:17 Dwaipayan Ray
2020-11-21 20:05 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Dwaipayan Ray @ 2020-11-21 16:17 UTC (permalink / raw)
To: joe; +Cc: dwaipayanray1, linux-kernel-mentees, linux-kernel
Provide fix option to INCLUDE_LINUX check to replace asm
includes.
Macros of type:
#include <asm/percpu.h>
are corrected to:
#include <linux/percpu.h>
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0da6422cd0fd..f852f130e065 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5468,8 +5468,11 @@ sub process {
CHK("ARCH_INCLUDE_LINUX",
"Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
} else {
- WARN("INCLUDE_LINUX",
- "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
+ if (WARN("INCLUDE_LINUX",
+ "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\<asm\/$file\>/\<linux\/$file\>/;
+ }
}
}
}
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-21 20:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-21 8:09 [Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX Dwaipayan Ray
2020-11-21 9:45 ` Lukas Bulwahn
2020-11-21 11:55 ` Dwaipayan Ray
2020-11-21 13:12 ` Lukas Bulwahn
2020-11-21 16:16 ` Dwaipayan Ray
2020-11-21 16:17 Dwaipayan Ray
2020-11-21 20:05 ` Joe Perches
2020-11-21 20:31 ` Dwaipayan Ray
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.