2024-05-14 13:29:31

by Jinjie Ruan

[permalink] [raw]
Subject: [PATCH] genirq: Refactor the irq_chip_xxx_parent()

Refactor the irq_chip_xxx_parent(), which can reduce a few lines of codes,
with no functional changes.

Signed-off-by: Jinjie Ruan <[email protected]>
---
kernel/irq/chip.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index dc94e0bf2c94..0e358d9a5d06 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1345,6 +1345,13 @@ int irq_chip_get_parent_state(struct irq_data *data,
}
EXPORT_SYMBOL_GPL(irq_chip_get_parent_state);

+static inline void irq_chip_do_common_parent(void (*chip_action)(struct irq_data *),
+ struct irq_data *data)
+{
+ data = data->parent_data;
+ chip_action(data);
+}
+
/**
* irq_chip_enable_parent - Enable the parent interrupt (defaults to unmask if
* NULL)
@@ -1352,11 +1359,8 @@ EXPORT_SYMBOL_GPL(irq_chip_get_parent_state);
*/
void irq_chip_enable_parent(struct irq_data *data)
{
- data = data->parent_data;
- if (data->chip->irq_enable)
- data->chip->irq_enable(data);
- else
- data->chip->irq_unmask(data);
+ irq_chip_do_common_parent(data->chip->irq_enable ? : data->chip->irq_unmask,
+ data);
}
EXPORT_SYMBOL_GPL(irq_chip_enable_parent);

@@ -1367,11 +1371,8 @@ EXPORT_SYMBOL_GPL(irq_chip_enable_parent);
*/
void irq_chip_disable_parent(struct irq_data *data)
{
- data = data->parent_data;
- if (data->chip->irq_disable)
- data->chip->irq_disable(data);
- else
- data->chip->irq_mask(data);
+ irq_chip_do_common_parent(data->chip->irq_disable ? : data->chip->irq_mask,
+ data);
}
EXPORT_SYMBOL_GPL(irq_chip_disable_parent);

@@ -1381,8 +1382,7 @@ EXPORT_SYMBOL_GPL(irq_chip_disable_parent);
*/
void irq_chip_ack_parent(struct irq_data *data)
{
- data = data->parent_data;
- data->chip->irq_ack(data);
+ irq_chip_do_common_parent(data->chip->irq_ack, data);
}
EXPORT_SYMBOL_GPL(irq_chip_ack_parent);

@@ -1392,8 +1392,7 @@ EXPORT_SYMBOL_GPL(irq_chip_ack_parent);
*/
void irq_chip_mask_parent(struct irq_data *data)
{
- data = data->parent_data;
- data->chip->irq_mask(data);
+ irq_chip_do_common_parent(data->chip->irq_mask, data);
}
EXPORT_SYMBOL_GPL(irq_chip_mask_parent);

@@ -1403,8 +1402,7 @@ EXPORT_SYMBOL_GPL(irq_chip_mask_parent);
*/
void irq_chip_mask_ack_parent(struct irq_data *data)
{
- data = data->parent_data;
- data->chip->irq_mask_ack(data);
+ irq_chip_do_common_parent(data->chip->irq_mask_ack, data);
}
EXPORT_SYMBOL_GPL(irq_chip_mask_ack_parent);

@@ -1414,8 +1412,7 @@ EXPORT_SYMBOL_GPL(irq_chip_mask_ack_parent);
*/
void irq_chip_unmask_parent(struct irq_data *data)
{
- data = data->parent_data;
- data->chip->irq_unmask(data);
+ irq_chip_do_common_parent(data->chip->irq_unmask, data);
}
EXPORT_SYMBOL_GPL(irq_chip_unmask_parent);

@@ -1425,8 +1422,7 @@ EXPORT_SYMBOL_GPL(irq_chip_unmask_parent);
*/
void irq_chip_eoi_parent(struct irq_data *data)
{
- data = data->parent_data;
- data->chip->irq_eoi(data);
+ irq_chip_do_common_parent(data->chip->irq_eoi, data);
}
EXPORT_SYMBOL_GPL(irq_chip_eoi_parent);

--
2.34.1



2024-05-14 17:26:59

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] genirq: Refactor the irq_chip_xxx_parent()

On Tue, May 14 2024 at 21:19, Jinjie Ruan wrote:
> +static inline void irq_chip_do_common_parent(void (*chip_action)(struct irq_data *),
> + struct irq_data *data)
> +{
> + data = data->parent_data;
> + chip_action(data);
> +}

> void irq_chip_eoi_parent(struct irq_data *data)
> {
> - data = data->parent_data;
> - data->chip->irq_eoi(data);
> + irq_chip_do_common_parent(data->chip->irq_eoi, data);
> }
> EXPORT_SYMBOL_GPL(irq_chip_eoi_parent);

How is this equivalent?

The original code does:

data = data->parent_data;
data->chip->irq_eoi(data);

which is equivalent to:

data->parent_data->chip->irq_eoi(data->parent_data);

while your change resolves to:

data->chip->irq_eoi(data->parent_data);

Seriously?

I'm starting to get tired of your flood of half baken 'cleanup' patches.

Thanks,

tglx

2024-05-16 15:07:42

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] genirq: Refactor the irq_chip_xxx_parent()



Hello,

kernel test robot noticed "BUG:kernel_NULL_pointer_dereference,address" on:

commit: d3e17411835053b9cd90e3ec2a3cff4e9eea55fa ("[PATCH] genirq: Refactor the irq_chip_xxx_parent()")
url: https://github.com/intel-lab-lkp/linux/commits/Jinjie-Ruan/genirq-Refactor-the-irq_chip_xxx_parent/20240514-213011
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 382d2ffe86efb1e2fa803d2cf17e5bfc34e574f3
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH] genirq: Refactor the irq_chip_xxx_parent()

in testcase: boot

compiler: gcc-13
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+-------------------------------------------------------+------------+------------+
| | 382d2ffe86 | d3e1741183 |
+-------------------------------------------------------+------------+------------+
| boot_successes | 18 | 0 |
| boot_failures | 0 | 18 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 18 |
| Oops:#[##] | 0 | 18 |
| RIP:apic_ack_edge | 0 | 18 |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 0 | 18 |
+-------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]


[ 0.414620][ C0] BUG: kernel NULL pointer dereference, address: 0000000000000030
[ 0.414623][ C0] #PF: supervisor read access in kernel mode
[ 0.414625][ C0] #PF: error_code(0x0000) - not-present page
[ 0.414627][ C0] PGD 0 P4D 0
[ 0.414630][ C0] Oops: 0000 [#1] SMP PTI
[ 0.414633][ C0] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.9.0-rc1-00052-gd3e174118350 #1
[ 0.414637][ C0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 0.414639][ C0] RIP: 0010:apic_ack_edge (arch/x86/kernel/apic/vector.c:1058 (discriminator 1) arch/x86/kernel/apic/vector.c:915 (discriminator 1))
[ 0.414646][ C0] Code: 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 53 48 89 fb 48 85 ff 74 4b 48 89 f8 48 89 c2 48 8b 40 28 48 85 c0 75 f4 48 8b 7a 30 <f6> 47 30 01 75 11 48 8b 43 10 8b 00 f6 c4 01 75 19 5b e9 04 12 01
All code
========
0: 90 nop
1: 90 nop
2: 90 nop
3: 90 nop
4: 90 nop
5: f3 0f 1e fa endbr64
9: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
e: 53 push %rbx
f: 48 89 fb mov %rdi,%rbx
12: 48 85 ff test %rdi,%rdi
15: 74 4b je 0x62
17: 48 89 f8 mov %rdi,%rax
1a: 48 89 c2 mov %rax,%rdx
1d: 48 8b 40 28 mov 0x28(%rax),%rax
21: 48 85 c0 test %rax,%rax
24: 75 f4 jne 0x1a
26: 48 8b 7a 30 mov 0x30(%rdx),%rdi
2a:* f6 47 30 01 testb $0x1,0x30(%rdi) <-- trapping instruction
2e: 75 11 jne 0x41
30: 48 8b 43 10 mov 0x10(%rbx),%rax
34: 8b 00 mov (%rax),%eax
36: f6 c4 01 test $0x1,%ah
39: 75 19 jne 0x54
3b: 5b pop %rbx
3c: e9 .byte 0xe9
3d: 04 12 add $0x12,%al
3f: 01 .byte 0x1

Code starting with the faulting instruction
===========================================
0: f6 47 30 01 testb $0x1,0x30(%rdi)
4: 75 11 jne 0x17
6: 48 8b 43 10 mov 0x10(%rbx),%rax
a: 8b 00 mov (%rax),%eax
c: f6 c4 01 test $0x1,%ah
f: 75 19 jne 0x2a
11: 5b pop %rbx
12: e9 .byte 0xe9
13: 04 12 add $0x12,%al
15: 01 .byte 0x1
[ 0.414649][ C0] RSP: 0000:ffffa39080003fa8 EFLAGS: 00010046
[ 0.414652][ C0] RAX: ffffffffb36738d0 RBX: 0000000000000000 RCX: 0000000046ebfda4
[ 0.414655][ C0] RDX: 0000000000000000 RSI: 0000000000000030 RDI: 0000000000000000
[ 0.414657][ C0] RBP: ffff961d001644a4 R08: 0000000000000001 R09: 0000000000000000
[ 0.414658][ C0] R10: 0000000000000000 R11: ffffa39080003ff8 R12: ffff961d00164428
[ 0.414660][ C0] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 0.414664][ C0] FS: 0000000000000000(0000) GS:ffff96202fc00000(0000) knlGS:0000000000000000
[ 0.414667][ C0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.414669][ C0] CR2: 0000000000000030 CR3: 00000001aba1c000 CR4: 00000000000000b0
[ 0.414671][ C0] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 0.414672][ C0] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 0.414674][ C0] Call Trace:
[ 0.414688][ C0] <IRQ>
[ 0.414690][ C0] ? __die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434)
[ 0.414696][ C0] ? page_fault_oops (arch/x86/mm/fault.c:713)
[ 0.414702][ C0] ? exc_page_fault (arch/x86/include/asm/irqflags.h:37 arch/x86/include/asm/irqflags.h:72 arch/x86/mm/fault.c:1513 arch/x86/mm/fault.c:1563)
[ 0.414707][ C0] ? asm_exc_page_fault (arch/x86/include/asm/idtentry.h:623)
[ 0.414712][ C0] ? __pfx_apic_ack_edge (arch/x86/kernel/apic/vector.c:914)
[ 0.414715][ C0] ? apic_ack_edge (arch/x86/kernel/apic/vector.c:1058 (discriminator 1) arch/x86/kernel/apic/vector.c:915 (discriminator 1))
[ 0.414718][ C0] handle_edge_irq (kernel/irq/chip.c:812)
[ 0.414724][ C0] __common_interrupt (include/asm-generic/irq_regs.h:29 (discriminator 5) arch/x86/kernel/irq.c:270 (discriminator 5))
[ 0.414730][ C0] common_interrupt (arch/x86/kernel/irq.c:247 (discriminator 35))
[ 0.414735][ C0] </IRQ>
[ 0.414736][ C0] <TASK>
[ 0.414737][ C0] asm_common_interrupt (arch/x86/include/asm/idtentry.h:693)
[ 0.414740][ C0] RIP: 0010:console_flush_all (kernel/printk/printk.c:2979)
[ 0.414744][ C0] Code: b5 88 54 24 07 e8 5c af 01 00 0f b6 54 24 07 41 88 14 24 e8 0e 1d 00 00 f7 c5 00 02 00 00 0f 84 da 00 00 00 fb 41 80 3c 24 00 <0f> 85 a3 00 00 00 49 8b 47 58 49 39 45 00 73 04 49 89 45 00 8b 05
All code
========
0: b5 88 mov $0x88,%ch
2: 54 push %rsp
3: 24 07 and $0x7,%al
5: e8 5c af 01 00 callq 0x1af66
a: 0f b6 54 24 07 movzbl 0x7(%rsp),%edx
f: 41 88 14 24 mov %dl,(%r12)
13: e8 0e 1d 00 00 callq 0x1d26
18: f7 c5 00 02 00 00 test $0x200,%ebp
1e: 0f 84 da 00 00 00 je 0xfe
24: fb sti
25: 41 80 3c 24 00 cmpb $0x0,(%r12)
2a:* 0f 85 a3 00 00 00 jne 0xd3 <-- trapping instruction
30: 49 8b 47 58 mov 0x58(%r15),%rax
34: 49 39 45 00 cmp %rax,0x0(%r13)
38: 73 04 jae 0x3e
3a: 49 89 45 00 mov %rax,0x0(%r13)
3e: 8b .byte 0x8b
3f: 05 .byte 0x5

Code starting with the faulting instruction
===========================================
0: 0f 85 a3 00 00 00 jne 0xa9
6: 49 8b 47 58 mov 0x58(%r15),%rax
a: 49 39 45 00 cmp %rax,0x0(%r13)
e: 73 04 jae 0x14
10: 49 89 45 00 mov %rax,0x0(%r13)
14: 8b .byte 0x8b
15: 05 .byte 0x5
[ 0.414747][ C0] RSP: 0000:ffffffffb5203d70 EFLAGS: 00000246
[ 0.414749][ C0] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000000
[ 0.414750][ C0] RDX: 0000000000000000 RSI: 0000000000000046 RDI: ffffffffb6214958
[ 0.414752][ C0] RBP: 0000000000000246 R08: 342e30202020205b R09: 205b5d3836303331
[ 0.414754][ C0] R10: 5b5d383630333134 R11: 205d305420202020 R12: ffffffffb5203ddf
[ 0.414756][ C0] R13: ffffffffb5203de0 R14: 0000000000000000 R15: ffffffffb56e53a0
[ 0.414761][ C0] console_unlock (kernel/printk/printk.c:3043)
[ 0.414764][ C0] vprintk_emit (kernel/printk/printk.c:3900 (discriminator 1) kernel/printk/printk.c:3935 (discriminator 1) kernel/printk/printk.c:2349 (discriminator 1))
[ 0.414767][ C0] _printk (kernel/printk/printk.c:2371)
[ 0.414773][ C0] __clocksource_register_scale (kernel/time/clocksource.c:1226)
[ 0.414778][ C0] tsc_init (arch/x86/kernel/tsc.c:1079 arch/x86/kernel/tsc.c:1620)
[ 0.414783][ C0] x86_late_time_init (arch/x86/include/asm/cpufeature.h:171 arch/x86/kernel/time.c:103)
[ 0.414787][ C0] start_kernel (init/main.c:1037)
[ 0.414792][ C0] x86_64_start_reservations (arch/x86/kernel/head64.c:495)
[ 0.414795][ C0] x86_64_start_kernel (arch/x86/kernel/head64.c:437 (discriminator 5))
[ 0.414798][ C0] common_startup_64 (arch/x86/kernel/head_64.S:421)
[ 0.414805][ C0] </TASK>
[ 0.414806][ C0] Modules linked in:
[ 0.414809][ C0] CR2: 0000000000000030
[ 0.414810][ C0] ---[ end trace 0000000000000000 ]---
[ 0.414812][ C0] RIP: 0010:apic_ack_edge (arch/x86/kernel/apic/vector.c:1058 (discriminator 1) arch/x86/kernel/apic/vector.c:915 (discriminator 1))
[ 0.414815][ C0] Code: 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 53 48 89 fb 48 85 ff 74 4b 48 89 f8 48 89 c2 48 8b 40 28 48 85 c0 75 f4 48 8b 7a 30 <f6> 47 30 01 75 11 48 8b 43 10 8b 00 f6 c4 01 75 19 5b e9 04 12 01
All code
========
0: 90 nop
1: 90 nop
2: 90 nop
3: 90 nop
4: 90 nop
5: f3 0f 1e fa endbr64
9: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
e: 53 push %rbx
f: 48 89 fb mov %rdi,%rbx
12: 48 85 ff test %rdi,%rdi
15: 74 4b je 0x62
17: 48 89 f8 mov %rdi,%rax
1a: 48 89 c2 mov %rax,%rdx
1d: 48 8b 40 28 mov 0x28(%rax),%rax
21: 48 85 c0 test %rax,%rax
24: 75 f4 jne 0x1a
26: 48 8b 7a 30 mov 0x30(%rdx),%rdi
2a:* f6 47 30 01 testb $0x1,0x30(%rdi) <-- trapping instruction
2e: 75 11 jne 0x41


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240516/[email protected]



--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki