[Linux-kernel-mentees] [PATCH] checkpatch: add --fix option for INCLUDE_LINUX
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?
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:
int register_kprobe(struct kprobe *kp);
int pre_handler(struct kprobe *p, struct pt_regs *regs);
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:
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
So I think it means that future additings should use <linux/pgtable.h>
I think the ARCH_INCLUDE_LINUX also deserves a --fix option similarly.
What do you think? Should I continue with this?
More information about the Linux-kernel-mentees