[Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX

Dwaipayan Ray dwaipayanray1 at gmail.com
Sat Nov 21 11:55:46 UTC 2020


On Sat, Nov 21, 2020 at 3:15 PM Lukas Bulwahn <lukas.bulwahn at gmail.com> wrote:
>
> On Sat, Nov 21, 2020 at 9:09 AM Dwaipayan Ray <dwaipayanray1 at 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.


More information about the Linux-kernel-mentees mailing list