All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
       [not found] <CGME20230424003252epcas2p29758e056b4766e53c252b5927a0cb406@epcas2p2.samsung.com>
@ 2023-04-24  1:04 ` Youngmin Nam
  2023-04-24  6:48   ` Dmitry Vyukov
                     ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Youngmin Nam @ 2023-04-24  1:04 UTC (permalink / raw)
  To: catalin.marinas, will
  Cc: mark.rutland, anshuman.khandual, broonie, alexandru.elisei, ardb,
	linux-arm-kernel, hy50.seo, youngmin.nam, dvyukov, andreyknvl

filter_irq_stacks() is supposed to cut entries which are related irq entries
from its call stack.
And in_irqentry_text() which is called by filter_irq_stacks()
uses __irqentry_text_start/end symbol to find irq entries in callstack.

But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t

This problem can makes unintentional deep call stack entries especially
in KASAN enabled situation as below.

[ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
[ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
[ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
[ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
[ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
[ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
[ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
[ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
[ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
[ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
[ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
[ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
[ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
[ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
[ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
[ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
[ 2479.386231]I[0:launcher-loader: 1719] Call trace:
[ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
[ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
[ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
[ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
[ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
[ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
[ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
[ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
[ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
[ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
[ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
[ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
[ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
[ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
[ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
[ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
[ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
[ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
[ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
[ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
[ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
[ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
[ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
[ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
[ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
[ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
[ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
[ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
[ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
[ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
[ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
[ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
[ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
[ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
[ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
[ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
[ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
[ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
[ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
[ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
[ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
[ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
[ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
[ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
[ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
[ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
[ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
[ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
[ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
[ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
[ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
[ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
[ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
[ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
[ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
[ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
[ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
[ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
[ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
[ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
[ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
[ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
[ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
[ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
[ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
[ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
[ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
[ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
[ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...

So let's set __exception_irq_entry with __irq_entry as a default.
Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.

* Before
ffffffc008010000 T __do_softirq
ffffffc008010000 T __irqentry_text_end
ffffffc008010000 T __irqentry_text_start
ffffffc008010000 T __softirqentry_text_start
ffffffc008010000 T _stext
ffffffc00801066c T __softirqentry_text_end
ffffffc008010670 T __entry_text_start

* After
ffffffc008010000 T __irqentry_text_start
ffffffc008010000 T _stext
ffffffc008010000 t gic_handle_irq
ffffffc00801013c t gic_handle_irq
ffffffc008010294 T __irqentry_text_end
ffffffc008010298 T __do_softirq
ffffffc008010298 T __softirqentry_text_start
ffffffc008010904 T __softirqentry_text_end
ffffffc008010908 T __entry_text_start

Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
---
 arch/arm64/include/asm/exception.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 19713d0f013b..18dbb35a337f 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -8,16 +8,11 @@
 #define __ASM_EXCEPTION_H
 
 #include <asm/esr.h>
-#include <asm/kprobes.h>
 #include <asm/ptrace.h>
 
 #include <linux/interrupt.h>
 
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #define __exception_irq_entry	__irq_entry
-#else
-#define __exception_irq_entry	__kprobes
-#endif
 
 static inline unsigned long disr_to_esr(u64 disr)
 {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24  1:04 ` [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Youngmin Nam
@ 2023-04-24  6:48   ` Dmitry Vyukov
  2023-04-24 11:01   ` Mark Rutland
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Dmitry Vyukov @ 2023-04-24  6:48 UTC (permalink / raw)
  To: Youngmin Nam
  Cc: catalin.marinas, will, mark.rutland, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl

On Mon, 24 Apr 2023 at 02:32, Youngmin Nam <youngmin.nam@samsung.com> wrote:
>
> filter_irq_stacks() is supposed to cut entries which are related irq entries
> from its call stack.
> And in_irqentry_text() which is called by filter_irq_stacks()
> uses __irqentry_text_start/end symbol to find irq entries in callstack.
>
> But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t

May be useful to add a check for non-empty irq/softirq entry sections
somewhere, otherwise it may silently break tomorrow and nobody will
notice. And as you see, failure mode is non-obvious and debugging is
painful.

But not sure where exactly. Maybe in stack_depot_early_init? Also it
may not yet be implemented for all arches, so the check needs to be
careful.

> This problem can makes unintentional deep call stack entries especially
> in KASAN enabled situation as below.
>
> [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
> [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...
>
> So let's set __exception_irq_entry with __irq_entry as a default.
> Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
>
> * Before
> ffffffc008010000 T __do_softirq
> ffffffc008010000 T __irqentry_text_end
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T __softirqentry_text_start
> ffffffc008010000 T _stext
> ffffffc00801066c T __softirqentry_text_end
> ffffffc008010670 T __entry_text_start
>
> * After
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T _stext
> ffffffc008010000 t gic_handle_irq
> ffffffc00801013c t gic_handle_irq
> ffffffc008010294 T __irqentry_text_end
> ffffffc008010298 T __do_softirq
> ffffffc008010298 T __softirqentry_text_start
> ffffffc008010904 T __softirqentry_text_end
> ffffffc008010908 T __entry_text_start
>
> Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
> ---
>  arch/arm64/include/asm/exception.h | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index 19713d0f013b..18dbb35a337f 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -8,16 +8,11 @@
>  #define __ASM_EXCEPTION_H
>
>  #include <asm/esr.h>
> -#include <asm/kprobes.h>
>  #include <asm/ptrace.h>
>
>  #include <linux/interrupt.h>
>
> -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  #define __exception_irq_entry  __irq_entry
> -#else
> -#define __exception_irq_entry  __kprobes
> -#endif
>
>  static inline unsigned long disr_to_esr(u64 disr)
>  {
> --
> 2.39.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24  1:04 ` [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Youngmin Nam
  2023-04-24  6:48   ` Dmitry Vyukov
@ 2023-04-24 11:01   ` Mark Rutland
  2023-04-24 12:09     ` Dmitry Vyukov
  2023-06-08  9:03   ` Mark Rutland
  2023-06-08 18:17   ` Catalin Marinas
  3 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2023-04-24 11:01 UTC (permalink / raw)
  To: Youngmin Nam
  Cc: catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, dvyukov,
	andreyknvl, maz

On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> filter_irq_stacks() is supposed to cut entries which are related irq entries
> from its call stack.
> And in_irqentry_text() which is called by filter_irq_stacks()
> uses __irqentry_text_start/end symbol to find irq entries in callstack.
> 
> But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> between __irqentry_text_start and __irqentry_text_end as we discussed in below link.

TBH, the __irqentry_text annotations don't make much sense, and I'd love to
remove them.

The irqchip handlers are not the actual exception entry points, and we invoke a
fair amount of code between those and the actual IRQ handlers (e.g. to map from
the irq domain to the actual hander, which might involve poking chained irqchip
handlers), so it doesn't make much sense for the irqchip handlers to be
special.

> https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> 
> This problem can makes unintentional deep call stack entries especially
> in KASAN enabled situation as below.

What exactly does KASAN need here? Is this just to limit the depth of the
trace?

If so, we could easily add an API to get a stacktrace up to an IRQ exception
boundary. IIRC we'd been asked for that in the past, and it's relatively simple
to implement that regardless of CONFIG_FUNCTION_GRAPH_TRACER.

> [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
> [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...
> 
> So let's set __exception_irq_entry with __irq_entry as a default.
> Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
> 
> * Before
> ffffffc008010000 T __do_softirq
> ffffffc008010000 T __irqentry_text_end
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T __softirqentry_text_start
> ffffffc008010000 T _stext
> ffffffc00801066c T __softirqentry_text_end
> ffffffc008010670 T __entry_text_start
> 
> * After
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T _stext
> ffffffc008010000 t gic_handle_irq
> ffffffc00801013c t gic_handle_irq
> ffffffc008010294 T __irqentry_text_end
> ffffffc008010298 T __do_softirq
> ffffffc008010298 T __softirqentry_text_start
> ffffffc008010904 T __softirqentry_text_end
> ffffffc008010908 T __entry_text_start
> 
> Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2

This change-id is meaningless upstream.

> ---
>  arch/arm64/include/asm/exception.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index 19713d0f013b..18dbb35a337f 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -8,16 +8,11 @@
>  #define __ASM_EXCEPTION_H
>  
>  #include <asm/esr.h>
> -#include <asm/kprobes.h>
>  #include <asm/ptrace.h>
>  
>  #include <linux/interrupt.h>
>  
> -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  #define __exception_irq_entry	__irq_entry
> -#else
> -#define __exception_irq_entry	__kprobes
> -#endif

How does this affect ftrace and kprobes? The commit message never explained why
this change is safe.

Thanks,
Mark.

>  
>  static inline unsigned long disr_to_esr(u64 disr)
>  {
> -- 
> 2.39.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24 11:01   ` Mark Rutland
@ 2023-04-24 12:09     ` Dmitry Vyukov
  2023-04-24 13:08       ` Mark Rutland
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Vyukov @ 2023-04-24 12:09 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Youngmin Nam, catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl,
	maz, kasan-dev

On Mon, 24 Apr 2023 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> > filter_irq_stacks() is supposed to cut entries which are related irq entries
> > from its call stack.
> > And in_irqentry_text() which is called by filter_irq_stacks()
> > uses __irqentry_text_start/end symbol to find irq entries in callstack.
> >
> > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> > between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
>
> TBH, the __irqentry_text annotations don't make much sense, and I'd love to
> remove them.
>
> The irqchip handlers are not the actual exception entry points, and we invoke a
> fair amount of code between those and the actual IRQ handlers (e.g. to map from
> the irq domain to the actual hander, which might involve poking chained irqchip
> handlers), so it doesn't make much sense for the irqchip handlers to be
> special.
>
> > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> >
> > This problem can makes unintentional deep call stack entries especially
> > in KASAN enabled situation as below.
>
> What exactly does KASAN need here? Is this just to limit the depth of the
> trace?

No, it's not just depth. Any uses of stack depot need stable
repeatable traces, so that they are deduplicated well. For irq stacks
it means removing the random part where the interrupt is delivered.
Otherwise stack depot grows without limits and overflows.

We don't need the exact entry point for this. A frame "close enough"
may work well if there are no memory allocations/frees skipped.

> If so, we could easily add an API to get a stacktrace up to an IRQ exception
> boundary. IIRC we'd been asked for that in the past, and it's relatively simple
> to implement that regardless of CONFIG_FUNCTION_GRAPH_TRACER.
>
> > [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> > [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> > [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> > [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> > [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> > [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> > [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> > [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> > [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> > [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> > [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> > [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> > [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> > [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> > [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> > [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> > [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> > [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> > [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> > [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> > [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> > [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> > [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> > [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> > [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> > [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> > [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> > [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> > [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> > [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> > [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> > [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> > [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> > [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> > [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> > [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> > [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> > [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> > [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> > [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> > [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> > [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> > [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> > [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> > [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> > [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> > [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> > [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> > [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> > [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> > [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> > [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> > [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> > [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> > [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> > [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
> > [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> > [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> > [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> > [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> > [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> > [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> > [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> > [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> > [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> > [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> > [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> > [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> > [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> > [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> > [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> > [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> > [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> > [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> > [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> > [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> > [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> > [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> > [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> > [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> > [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> > [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> > [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> > [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...
> >
> > So let's set __exception_irq_entry with __irq_entry as a default.
> > Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
> >
> > * Before
> > ffffffc008010000 T __do_softirq
> > ffffffc008010000 T __irqentry_text_end
> > ffffffc008010000 T __irqentry_text_start
> > ffffffc008010000 T __softirqentry_text_start
> > ffffffc008010000 T _stext
> > ffffffc00801066c T __softirqentry_text_end
> > ffffffc008010670 T __entry_text_start
> >
> > * After
> > ffffffc008010000 T __irqentry_text_start
> > ffffffc008010000 T _stext
> > ffffffc008010000 t gic_handle_irq
> > ffffffc00801013c t gic_handle_irq
> > ffffffc008010294 T __irqentry_text_end
> > ffffffc008010298 T __do_softirq
> > ffffffc008010298 T __softirqentry_text_start
> > ffffffc008010904 T __softirqentry_text_end
> > ffffffc008010908 T __entry_text_start
> >
> > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
>
> This change-id is meaningless upstream.
>
> > ---
> >  arch/arm64/include/asm/exception.h | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> > index 19713d0f013b..18dbb35a337f 100644
> > --- a/arch/arm64/include/asm/exception.h
> > +++ b/arch/arm64/include/asm/exception.h
> > @@ -8,16 +8,11 @@
> >  #define __ASM_EXCEPTION_H
> >
> >  #include <asm/esr.h>
> > -#include <asm/kprobes.h>
> >  #include <asm/ptrace.h>
> >
> >  #include <linux/interrupt.h>
> >
> > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> >  #define __exception_irq_entry        __irq_entry
> > -#else
> > -#define __exception_irq_entry        __kprobes
> > -#endif
>
> How does this affect ftrace and kprobes? The commit message never explained why
> this change is safe.
>
> Thanks,
> Mark.
>
> >
> >  static inline unsigned long disr_to_esr(u64 disr)
> >  {
> > --
> > 2.39.2
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24 12:09     ` Dmitry Vyukov
@ 2023-04-24 13:08       ` Mark Rutland
  2023-04-25  2:31         ` Youngmin Nam
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2023-04-24 13:08 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Youngmin Nam, catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl,
	maz, kasan-dev

On Mon, Apr 24, 2023 at 02:09:05PM +0200, Dmitry Vyukov wrote:
> On Mon, 24 Apr 2023 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> > > filter_irq_stacks() is supposed to cut entries which are related irq entries
> > > from its call stack.
> > > And in_irqentry_text() which is called by filter_irq_stacks()
> > > uses __irqentry_text_start/end symbol to find irq entries in callstack.
> > >
> > > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> > > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> > > between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> >
> > TBH, the __irqentry_text annotations don't make much sense, and I'd love to
> > remove them.
> >
> > The irqchip handlers are not the actual exception entry points, and we invoke a
> > fair amount of code between those and the actual IRQ handlers (e.g. to map from
> > the irq domain to the actual hander, which might involve poking chained irqchip
> > handlers), so it doesn't make much sense for the irqchip handlers to be
> > special.
> >
> > > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> > >
> > > This problem can makes unintentional deep call stack entries especially
> > > in KASAN enabled situation as below.
> >
> > What exactly does KASAN need here? Is this just to limit the depth of the
> > trace?
> 
> No, it's not just depth. Any uses of stack depot need stable
> repeatable traces, so that they are deduplicated well. For irq stacks
> it means removing the random part where the interrupt is delivered.
> Otherwise stack depot grows without limits and overflows.

Sure -- you want to filter out the non-deterministic context that the interrupt
was taken *from*.

> We don't need the exact entry point for this. A frame "close enough"
> may work well if there are no memory allocations/frees skipped.

With that in mind, I think what we should do is cut this at the instant we
enter the exception; for the trace below that would be el1h_64_irq. I've added
some line spacing there to make it stand out.

That would mean that we'd have three entry points that an interrupt trace might
start from:

* el1h_64_irq()
* el0t_64_irq()
* el0t_32_irq()

... so we might have three traces for a given interrupt, but the portion
between that and the irqchip handler would be deterministic, so deduplication
would only end up with three traces.

It may be useful to distinguish the three cases, since some IRQ handlers do
different things when user_mode(regs) and/or compat_user_mode(regs) are true.

> > If so, we could easily add an API to get a stacktrace up to an IRQ exception
> > boundary. IIRC we'd been asked for that in the past, and it's relatively simple
> > to implement that regardless of CONFIG_FUNCTION_GRAPH_TRACER.
> >
> > > [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> > > [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> > > [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> > > [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> > > [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> > > [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> > > [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> > > [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> > > [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> > > [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> > > [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> > > [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> > > [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> > > [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> > > [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> > > [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> > > [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> > > [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> > > [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> > > [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> > > [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> > > [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> > > [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> > > [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> > > [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> > > [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> > > [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> > > [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> > > [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> > > [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> > > [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > > [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> > > [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> > > [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> > > [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > > [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> > > [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> > > [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> > > [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> > > [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> > > [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> > > [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> > > [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> > > [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> > > [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> > > [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> > > [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> > > [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> > > [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> > > [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> > > [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> > > [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> > > [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> > > [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> > > [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> > > [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> > > [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> > > [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c

This is where we'd cut the trace with my suggestion.

> > > [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> > > [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> > > [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> > > [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> > > [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> > > [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> > > [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> > > [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> > > [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> > > [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> > > [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> > > [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> > > [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> > > [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> > > [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> > > [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> > > [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> > > [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> > > [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> > > [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> > > [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> > > [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> > > [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> > > [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> > > [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> > > [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> > > [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> > > [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...

Thanks,
Mark.

> > >
> > > So let's set __exception_irq_entry with __irq_entry as a default.
> > > Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
> > >
> > > * Before
> > > ffffffc008010000 T __do_softirq
> > > ffffffc008010000 T __irqentry_text_end
> > > ffffffc008010000 T __irqentry_text_start
> > > ffffffc008010000 T __softirqentry_text_start
> > > ffffffc008010000 T _stext
> > > ffffffc00801066c T __softirqentry_text_end
> > > ffffffc008010670 T __entry_text_start
> > >
> > > * After
> > > ffffffc008010000 T __irqentry_text_start
> > > ffffffc008010000 T _stext
> > > ffffffc008010000 t gic_handle_irq
> > > ffffffc00801013c t gic_handle_irq
> > > ffffffc008010294 T __irqentry_text_end
> > > ffffffc008010298 T __do_softirq
> > > ffffffc008010298 T __softirqentry_text_start
> > > ffffffc008010904 T __softirqentry_text_end
> > > ffffffc008010908 T __entry_text_start
> > >
> > > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > > Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
> >
> > This change-id is meaningless upstream.
> >
> > > ---
> > >  arch/arm64/include/asm/exception.h | 5 -----
> > >  1 file changed, 5 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> > > index 19713d0f013b..18dbb35a337f 100644
> > > --- a/arch/arm64/include/asm/exception.h
> > > +++ b/arch/arm64/include/asm/exception.h
> > > @@ -8,16 +8,11 @@
> > >  #define __ASM_EXCEPTION_H
> > >
> > >  #include <asm/esr.h>
> > > -#include <asm/kprobes.h>
> > >  #include <asm/ptrace.h>
> > >
> > >  #include <linux/interrupt.h>
> > >
> > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > >  #define __exception_irq_entry        __irq_entry
> > > -#else
> > > -#define __exception_irq_entry        __kprobes
> > > -#endif
> >
> > How does this affect ftrace and kprobes? The commit message never explained why
> > this change is safe.
> >
> > Thanks,
> > Mark.
> >
> > >
> > >  static inline unsigned long disr_to_esr(u64 disr)
> > >  {
> > > --
> > > 2.39.2
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24 13:08       ` Mark Rutland
@ 2023-04-25  2:31         ` Youngmin Nam
  2023-04-25 13:39           ` Mark Rutland
  0 siblings, 1 reply; 11+ messages in thread
From: Youngmin Nam @ 2023-04-25  2:31 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Youngmin Nam, catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl,
	maz, kasan-dev, Dmitry Vyukov, d7271.choe

[-- Attachment #1: Type: text/plain, Size: 13769 bytes --]

On Mon, Apr 24, 2023 at 02:08:14PM +0100, Mark Rutland wrote:
> On Mon, Apr 24, 2023 at 02:09:05PM +0200, Dmitry Vyukov wrote:
> > On Mon, 24 Apr 2023 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> > > > filter_irq_stacks() is supposed to cut entries which are related irq entries
> > > > from its call stack.
> > > > And in_irqentry_text() which is called by filter_irq_stacks()
> > > > uses __irqentry_text_start/end symbol to find irq entries in callstack.
> > > >
> > > > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> > > > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> > > > between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> > >
> > > TBH, the __irqentry_text annotations don't make much sense, and I'd love to
> > > remove them.
> > >
> > > The irqchip handlers are not the actual exception entry points, and we invoke a
> > > fair amount of code between those and the actual IRQ handlers (e.g. to map from
> > > the irq domain to the actual hander, which might involve poking chained irqchip
> > > handlers), so it doesn't make much sense for the irqchip handlers to be
> > > special.
> > >
> > > > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> > > >
> > > > This problem can makes unintentional deep call stack entries especially
> > > > in KASAN enabled situation as below.
> > >
> > > What exactly does KASAN need here? Is this just to limit the depth of the
> > > trace?
> > 
> > No, it's not just depth. Any uses of stack depot need stable
> > repeatable traces, so that they are deduplicated well. For irq stacks
> > it means removing the random part where the interrupt is delivered.
> > Otherwise stack depot grows without limits and overflows.

Hi Dmitry Vyukov.
Thanks for your additional comments.

> 
> Sure -- you want to filter out the non-deterministic context that the interrupt
> was taken *from*.
> 
> > We don't need the exact entry point for this. A frame "close enough"
> > may work well if there are no memory allocations/frees skipped.
> 
> With that in mind, I think what we should do is cut this at the instant we
> enter the exception; for the trace below that would be el1h_64_irq. I've added
> some line spacing there to make it stand out.
> 
> That would mean that we'd have three entry points that an interrupt trace might
> start from:
> 
> * el1h_64_irq()
> * el0t_64_irq()
> * el0t_32_irq()
>

Hi Mark.
Thanks for your kind review.

If I understand your intention corretly, I should add "__irq_entry"
to C function of irq_handler as below.

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
-asmlinkage void el1h_64_irq_handler(struct pt_regs *regs);
+asmlinkage void __irq_entry el1h_64_irq_handler(struct pt_regs *regs);

-asmlinkage void el0t_64_irq_handler(struct pt_regs *regs);
+asmlinkage void __irq_entry el0t_64_irq_handler(struct pt_regs *regs);

-asmlinkage void el0t_32_irq_handler(struct pt_regs *regs);
+asmlinkage void __irq_entry el0t_32_irq_handler(struct pt_regs *regs);

But these irq handlers are marked with "noinstr" already so that we can't put them into
irqentry section.

arch/arm64/kernel/entry-common.c:492:asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
arch/arm64/kernel/entry-common.c:730:asmlinkage void noinstr el0t_64_irq_handler(struct pt_regs *regs)
arch/arm64/kernel/entry-common.c:824:asmlinkage void noinstr el0t_32_irq_handler(struct pt_regs *regs)

Could you tell me that I am doing right ?

> ... so we might have three traces for a given interrupt, but the portion
> between that and the irqchip handler would be deterministic, so deduplication
> would only end up with three traces.
> 
> It may be useful to distinguish the three cases, since some IRQ handlers do
> different things when user_mode(regs) and/or compat_user_mode(regs) are true.
> 
> > > If so, we could easily add an API to get a stacktrace up to an IRQ exception
> > > boundary. IIRC we'd been asked for that in the past, and it's relatively simple
> > > to implement that regardless of CONFIG_FUNCTION_GRAPH_TRACER.
> > >
> > > > [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> > > > [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> > > > [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> > > > [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> > > > [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> > > > [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> > > > [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> > > > [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> > > > [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> > > > [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> > > > [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> > > > [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> > > > [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> > > > [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> > > > [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> > > > [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> > > > [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> > > > [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> > > > [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> > > > [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> > > > [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> > > > [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> > > > [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> > > > [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> > > > [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> > > > [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> > > > [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> > > > [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> > > > [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> > > > [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> > > > [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > > > [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> > > > [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> > > > [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> > > > [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> > > > [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> > > > [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> > > > [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> > > > [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> > > > [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> > > > [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> > > > [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> > > > [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> > > > [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> > > > [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> > > > [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> > > > [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> > > > [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> > > > [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> > > > [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> > > > [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> > > > [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> > > > [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> > > > [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> > > > [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> > > > [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> > > > [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> > > > [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
> 
> This is where we'd cut the trace with my suggestion.
> 
> > > > [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> > > > [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> > > > [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> > > > [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> > > > [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> > > > [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> > > > [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> > > > [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> > > > [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> > > > [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> > > > [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> > > > [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> > > > [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> > > > [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> > > > [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> > > > [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> > > > [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> > > > [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> > > > [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> > > > [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> > > > [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> > > > [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> > > > [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> > > > [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> > > > [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> > > > [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> > > > [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> > > > [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...
> 
> Thanks,
> Mark.
> 
> > > >
> > > > So let's set __exception_irq_entry with __irq_entry as a default.
> > > > Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
> > > >
> > > > * Before
> > > > ffffffc008010000 T __do_softirq
> > > > ffffffc008010000 T __irqentry_text_end
> > > > ffffffc008010000 T __irqentry_text_start
> > > > ffffffc008010000 T __softirqentry_text_start
> > > > ffffffc008010000 T _stext
> > > > ffffffc00801066c T __softirqentry_text_end
> > > > ffffffc008010670 T __entry_text_start
> > > >
> > > > * After
> > > > ffffffc008010000 T __irqentry_text_start
> > > > ffffffc008010000 T _stext
> > > > ffffffc008010000 t gic_handle_irq
> > > > ffffffc00801013c t gic_handle_irq
> > > > ffffffc008010294 T __irqentry_text_end
> > > > ffffffc008010298 T __do_softirq
> > > > ffffffc008010298 T __softirqentry_text_start
> > > > ffffffc008010904 T __softirqentry_text_end
> > > > ffffffc008010908 T __entry_text_start
> > > >
> > > > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> > > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > > > Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
> > >
> > > This change-id is meaningless upstream.
> > >
> > > > ---
> > > >  arch/arm64/include/asm/exception.h | 5 -----
> > > >  1 file changed, 5 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> > > > index 19713d0f013b..18dbb35a337f 100644
> > > > --- a/arch/arm64/include/asm/exception.h
> > > > +++ b/arch/arm64/include/asm/exception.h
> > > > @@ -8,16 +8,11 @@
> > > >  #define __ASM_EXCEPTION_H
> > > >
> > > >  #include <asm/esr.h>
> > > > -#include <asm/kprobes.h>
> > > >  #include <asm/ptrace.h>
> > > >
> > > >  #include <linux/interrupt.h>
> > > >
> > > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > > >  #define __exception_irq_entry        __irq_entry
> > > > -#else
> > > > -#define __exception_irq_entry        __kprobes
> > > > -#endif
> > >
> > > How does this affect ftrace and kprobes? The commit message never explained why
> > > this change is safe.
> > >
> > > Thanks,
> > > Mark.
> > >
> > > >
> > > >  static inline unsigned long disr_to_esr(u64 disr)
> > > >  {
> > > > --
> > > > 2.39.2
> > > >
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-25  2:31         ` Youngmin Nam
@ 2023-04-25 13:39           ` Mark Rutland
  2023-04-26  5:06             ` Youngmin Nam
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2023-04-25 13:39 UTC (permalink / raw)
  To: Youngmin Nam
  Cc: catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl,
	maz, kasan-dev, Dmitry Vyukov, d7271.choe

On Tue, Apr 25, 2023 at 11:31:31AM +0900, Youngmin Nam wrote:
> On Mon, Apr 24, 2023 at 02:08:14PM +0100, Mark Rutland wrote:
> > On Mon, Apr 24, 2023 at 02:09:05PM +0200, Dmitry Vyukov wrote:
> > > On Mon, 24 Apr 2023 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
> > > >
> > > > On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> > > > > filter_irq_stacks() is supposed to cut entries which are related irq entries
> > > > > from its call stack.
> > > > > And in_irqentry_text() which is called by filter_irq_stacks()
> > > > > uses __irqentry_text_start/end symbol to find irq entries in callstack.
> > > > >
> > > > > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> > > > > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> > > > > between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> > > >
> > > > TBH, the __irqentry_text annotations don't make much sense, and I'd love to
> > > > remove them.
> > > >
> > > > The irqchip handlers are not the actual exception entry points, and we invoke a
> > > > fair amount of code between those and the actual IRQ handlers (e.g. to map from
> > > > the irq domain to the actual hander, which might involve poking chained irqchip
> > > > handlers), so it doesn't make much sense for the irqchip handlers to be
> > > > special.
> > > >
> > > > > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> > > > >
> > > > > This problem can makes unintentional deep call stack entries especially
> > > > > in KASAN enabled situation as below.
> > > >
> > > > What exactly does KASAN need here? Is this just to limit the depth of the
> > > > trace?
> > > 
> > > No, it's not just depth. Any uses of stack depot need stable
> > > repeatable traces, so that they are deduplicated well. For irq stacks
> > > it means removing the random part where the interrupt is delivered.
> > > Otherwise stack depot grows without limits and overflows.
> 
> Hi Dmitry Vyukov.
> Thanks for your additional comments.
> 
> > 
> > Sure -- you want to filter out the non-deterministic context that the interrupt
> > was taken *from*.
> > 
> > > We don't need the exact entry point for this. A frame "close enough"
> > > may work well if there are no memory allocations/frees skipped.
> > 
> > With that in mind, I think what we should do is cut this at the instant we
> > enter the exception; for the trace below that would be el1h_64_irq. I've added
> > some line spacing there to make it stand out.
> > 
> > That would mean that we'd have three entry points that an interrupt trace might
> > start from:
> > 
> > * el1h_64_irq()
> > * el0t_64_irq()
> > * el0t_32_irq()
> >
> 
> Hi Mark.
> Thanks for your kind review.
> 
> If I understand your intention corretly, I should add "__irq_entry"
> to C function of irq_handler as below.

I'd meant something like the below, marking the assembly (as x86 does) rather
than the C code. I'll try to sort that out and send a proper patch series after
-rc1.

Thanks,
Mark.

---->8----
From 7e54be3ea2420af348c710afab743b94ced72881 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Tue, 25 Apr 2023 14:13:39 +0100
Subject: [PATCH] WIP: arm64: entry: handle irqentry consistently

Follow the example of x86 in commit:

  f0178fc01fe46bab ("x86/entry: Unbreak __irqentry_text_start/end magic")

... and consistently treat the asm IRQ/FIQ entry points as irqentry, and
nothing else.

TODO:
* Explain stackdepot details
* Explain why dropping __kprobes is fine
* Explain why placing entry asm in .irqentry.text is fine.
* Explain why we don't need to mark ret_to_* or the actual vector
* Check how this works for ftrace

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/exception.h | 10 +++++-----
 arch/arm64/include/asm/irq.h       |  6 ++++++
 arch/arm64/kernel/entry.S          | 20 +++++++++++---------
 drivers/irqchip/irq-dw-apb-ictl.c  |  4 +++-
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 92963f98afece..6c74a8a3aad99 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -13,11 +13,11 @@
 
 #include <linux/interrupt.h>
 
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-#define __exception_irq_entry	__irq_entry
-#else
-#define __exception_irq_entry	__kprobes
-#endif
+/*
+ * irqchip drivers shared with arch/arm mark IRQ handlers with
+ * __exception_irq_entry. Make this a NOP for arm64.
+ */
+#define __exception_irq_entry
 
 static inline unsigned long disr_to_esr(u64 disr)
 {
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index fac08e18bcd51..a1af8fafc6cb5 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -6,6 +6,12 @@
 
 #include <asm-generic/irq.h>
 
+/*
+ * The irq entry code is in assembly. Make the build fail if
+ * something moves a C function into the __irq_entry section.
+ */
+#define __irq_entry __invalid_section
+
 struct pt_regs;
 
 int set_handle_irq(void (*handle_irq)(struct pt_regs *));
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index ab2a6e33c0528..a66cd0ce79956 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -562,7 +562,8 @@ SYM_CODE_END(__bad_stack)
 #endif /* CONFIG_VMAP_STACK */
 
 
-	.macro entry_handler el:req, ht:req, regsize:req, label:req
+	.macro entry_handler el:req, ht:req, regsize:req, label:req, section = ".entry.text"
+	.pushsection \section, "ax"
 SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label)
 	kernel_entry \el, \regsize
 	mov	x0, sp
@@ -573,29 +574,30 @@ SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label)
 	b	ret_to_kernel
 	.endif
 SYM_CODE_END(el\el\ht\()_\regsize\()_\label)
+	.popsection
 	.endm
 
 /*
  * Early exception handlers
  */
 	entry_handler	1, t, 64, sync
-	entry_handler	1, t, 64, irq
-	entry_handler	1, t, 64, fiq
+	entry_handler	1, t, 64, irq,	".irqentry.text"
+	entry_handler	1, t, 64, fiq,	".irqentry.text"
 	entry_handler	1, t, 64, error
 
 	entry_handler	1, h, 64, sync
-	entry_handler	1, h, 64, irq
-	entry_handler	1, h, 64, fiq
+	entry_handler	1, h, 64, irq,	".irqentry.text"
+	entry_handler	1, h, 64, fiq,	".irqentry.text"
 	entry_handler	1, h, 64, error
 
 	entry_handler	0, t, 64, sync
-	entry_handler	0, t, 64, irq
-	entry_handler	0, t, 64, fiq
+	entry_handler	0, t, 64, irq,	".irqentry.text"
+	entry_handler	0, t, 64, fiq,	".irqentry.text"
 	entry_handler	0, t, 64, error
 
 	entry_handler	0, t, 32, sync
-	entry_handler	0, t, 32, irq
-	entry_handler	0, t, 32, fiq
+	entry_handler	0, t, 32, irq,	".irqentry.text"
+	entry_handler	0, t, 32, fiq,	".irqentry.text"
 	entry_handler	0, t, 32, error
 
 SYM_CODE_START_LOCAL(ret_to_kernel)
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index d5c1c750c8d2d..ad315bdfc3ef6 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -19,6 +19,8 @@
 #include <linux/of_irq.h>
 #include <linux/interrupt.h>
 
+#include <asm/exception.h>
+
 #define APB_INT_ENABLE_L	0x00
 #define APB_INT_ENABLE_H	0x04
 #define APB_INT_MASK_L		0x08
@@ -30,7 +32,7 @@
 /* irq domain of the primary interrupt controller. */
 static struct irq_domain *dw_apb_ictl_irq_domain;
 
-static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
+static void __exception_irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
 {
 	struct irq_domain *d = dw_apb_ictl_irq_domain;
 	int n;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-25 13:39           ` Mark Rutland
@ 2023-04-26  5:06             ` Youngmin Nam
       [not found]               ` <ZF1+cLp7Io7L25yG@perf>
  0 siblings, 1 reply; 11+ messages in thread
From: Youngmin Nam @ 2023-04-26  5:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: catalin.marinas, will, anshuman.khandual, broonie,
	alexandru.elisei, ardb, linux-arm-kernel, hy50.seo, andreyknvl,
	maz, kasan-dev, Dmitry Vyukov, d7271.choe

[-- Attachment #1: Type: text/plain, Size: 6935 bytes --]

On Tue, Apr 25, 2023 at 02:39:51PM +0100, Mark Rutland wrote:
> On Tue, Apr 25, 2023 at 11:31:31AM +0900, Youngmin Nam wrote:
> > On Mon, Apr 24, 2023 at 02:08:14PM +0100, Mark Rutland wrote:
> > > On Mon, Apr 24, 2023 at 02:09:05PM +0200, Dmitry Vyukov wrote:
> > > > On Mon, 24 Apr 2023 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
> > > > >
> > > > > On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> > > > > > filter_irq_stacks() is supposed to cut entries which are related irq entries
> > > > > > from its call stack.
> > > > > > And in_irqentry_text() which is called by filter_irq_stacks()
> > > > > > uses __irqentry_text_start/end symbol to find irq entries in callstack.
> > > > > >
> > > > > > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> > > > > > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> > > > > > between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> > > > >
> > > > > TBH, the __irqentry_text annotations don't make much sense, and I'd love to
> > > > > remove them.
> > > > >
> > > > > The irqchip handlers are not the actual exception entry points, and we invoke a
> > > > > fair amount of code between those and the actual IRQ handlers (e.g. to map from
> > > > > the irq domain to the actual hander, which might involve poking chained irqchip
> > > > > handlers), so it doesn't make much sense for the irqchip handlers to be
> > > > > special.
> > > > >
> > > > > > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> > > > > >
> > > > > > This problem can makes unintentional deep call stack entries especially
> > > > > > in KASAN enabled situation as below.
> > > > >
> > > > > What exactly does KASAN need here? Is this just to limit the depth of the
> > > > > trace?
> > > > 
> > > > No, it's not just depth. Any uses of stack depot need stable
> > > > repeatable traces, so that they are deduplicated well. For irq stacks
> > > > it means removing the random part where the interrupt is delivered.
> > > > Otherwise stack depot grows without limits and overflows.
> > 
> > Hi Dmitry Vyukov.
> > Thanks for your additional comments.
> > 
> > > 
> > > Sure -- you want to filter out the non-deterministic context that the interrupt
> > > was taken *from*.
> > > 
> > > > We don't need the exact entry point for this. A frame "close enough"
> > > > may work well if there are no memory allocations/frees skipped.
> > > 
> > > With that in mind, I think what we should do is cut this at the instant we
> > > enter the exception; for the trace below that would be el1h_64_irq. I've added
> > > some line spacing there to make it stand out.
> > > 
> > > That would mean that we'd have three entry points that an interrupt trace might
> > > start from:
> > > 
> > > * el1h_64_irq()
> > > * el0t_64_irq()
> > > * el0t_32_irq()
> > >
> > 
> > Hi Mark.
> > Thanks for your kind review.
> > 
> > If I understand your intention corretly, I should add "__irq_entry"
> > to C function of irq_handler as below.
> 
> I'd meant something like the below, marking the assembly (as x86 does) rather
> than the C code. I'll try to sort that out and send a proper patch series after
> -rc1.
> 
> Thanks,
> Mark.
> 
After applying your draft patch,
I checked System.map and could see irq entries we expected were included as below.

ffffffc008000000 T _text
ffffffc008010000 T __irqentry_text_start
ffffffc008010000 T _stext
ffffffc008010000 t el1t_64_irq
ffffffc00801006c t el1t_64_fiq
ffffffc0080100d8 t el1h_64_irq
ffffffc008010144 t el1h_64_fiq
ffffffc0080101b0 t el0t_64_irq
ffffffc008010344 t el0t_64_fiq
ffffffc0080104d8 t el0t_32_irq
ffffffc008010670 t el0t_32_fiq
ffffffc008010928 T __do_softirq
ffffffc008010928 T __irqentry_text_end
ffffffc008010928 T __softirqentry_text_start
ffffffc008010fa0 T __entry_text_start
ffffffc008010fa0 T __softirqentry_text_end

And then, I confirmed callstack was cut correctly as below.

[   89.738326]I[5:NetworkWatchlis: 1084]  kasan_save_stack+0x40/0x70
[   89.738337]I[5:NetworkWatchlis: 1084]  save_stack_info+0x34/0x138
[   89.738348]I[5:NetworkWatchlis: 1084]  kasan_save_free_info+0x18/0x24
[   89.738358]I[5:NetworkWatchlis: 1084]  ____kasan_slab_free+0x16c/0x170
[   89.738369]I[5:NetworkWatchlis: 1084]  __kasan_slab_free+0x10/0x20
[   89.738379]I[5:NetworkWatchlis: 1084]  kmem_cache_free+0x238/0x53c
[   89.738388]I[5:NetworkWatchlis: 1084]  mempool_free_slab+0x1c/0x28
[   89.738397]I[5:NetworkWatchlis: 1084]  mempool_free+0x7c/0x1a0
[   89.738405]I[5:NetworkWatchlis: 1084]  bvec_free+0x34/0x80
[   89.738417]I[5:NetworkWatchlis: 1084]  bio_free+0x60/0x98
[   89.738426]I[5:NetworkWatchlis: 1084]  bio_put+0x50/0x21c
[   89.738434]I[5:NetworkWatchlis: 1084]  f2fs_write_end_io+0x4ac/0x4d0
[   89.738444]I[5:NetworkWatchlis: 1084]  bio_endio+0x2dc/0x300
[   89.738453]I[5:NetworkWatchlis: 1084]  __dm_io_complete+0x324/0x37c
[   89.738464]I[5:NetworkWatchlis: 1084]  dm_io_dec_pending+0x60/0xa4
[   89.738474]I[5:NetworkWatchlis: 1084]  clone_endio+0xf8/0x2f0
[   89.738484]I[5:NetworkWatchlis: 1084]  bio_endio+0x2dc/0x300
[   89.738493]I[5:NetworkWatchlis: 1084]  blk_update_request+0x258/0x63c
[   89.738503]I[5:NetworkWatchlis: 1084]  scsi_end_request+0x50/0x304
[   89.738514]I[5:NetworkWatchlis: 1084]  scsi_io_completion+0x88/0x160
[   89.738524]I[5:NetworkWatchlis: 1084]  scsi_finish_command+0x17c/0x194
[   89.738534]I[5:NetworkWatchlis: 1084]  scsi_complete+0xcc/0x158
[   89.738543]I[5:NetworkWatchlis: 1084]  blk_mq_complete_request+0x4c/0x5c
[   89.738553]I[5:NetworkWatchlis: 1084]  scsi_done_internal+0xf4/0x1e0
[   89.738564]I[5:NetworkWatchlis: 1084]  scsi_done+0x14/0x20
[   89.738575]I[5:NetworkWatchlis: 1084]  ufshcd_compl_one_cqe+0x578/0x71c
[   89.738585]I[5:NetworkWatchlis: 1084]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
[   89.738594]I[5:NetworkWatchlis: 1084]  exynos_vendor_mcq_irq+0xac/0xc4 [ufs_exynos_core]
[   89.738638]I[5:NetworkWatchlis: 1084]  __handle_irq_event_percpu+0xd0/0x348
[   89.738647]I[5:NetworkWatchlis: 1084]  handle_irq_event_percpu+0x24/0x74
[   89.738656]I[5:NetworkWatchlis: 1084]  handle_irq_event+0x74/0xe0
[   89.738665]I[5:NetworkWatchlis: 1084]  handle_fasteoi_irq+0x174/0x240
[   89.738675]I[5:NetworkWatchlis: 1084]  handle_irq_desc+0x6c/0x2c0
[   89.738686]I[5:NetworkWatchlis: 1084]  generic_handle_domain_irq+0x1c/0x28
[   89.738697]I[5:NetworkWatchlis: 1084]  gic_handle_irq+0x64/0x154
[   89.738707]I[5:NetworkWatchlis: 1084]  call_on_irq_stack+0x2c/0x54
[   89.738717]I[5:NetworkWatchlis: 1084]  do_interrupt_handler+0x70/0xa0
[   89.738726]I[5:NetworkWatchlis: 1084]  el1_interrupt+0x34/0x68
[   89.738737]I[5:NetworkWatchlis: 1084]  el1h_64_irq_handler+0x18/0x24
[   89.738747]I[5:NetworkWatchlis: 1084]  el1h_64_irq+0x68/0x6c

Thanks for your work.
Please add me when you send the final patch so that I can test again.

> ---->8----

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
       [not found]                 ` <ZF5gmBz4NbDseDHp@FVFF77S0Q05N>
@ 2023-05-25 23:20                   ` Youngmin Nam
  0 siblings, 0 replies; 11+ messages in thread
From: Youngmin Nam @ 2023-05-25 23:20 UTC (permalink / raw)
  To: Mark Rutland
  Cc: lexandru.elisei, andreyknvl, anshuman.khandual, ardb, broonie,
	catalin.marinas, d7271.choe, dvyukov, hy50.seo, kasan-dev,
	linux-arm-kernel, maz, will, youngmin.nam

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

On Fri, May 12, 2023 at 04:51:52PM +0100, Mark Rutland wrote:
> Hi,
> 
> On Fri, May 12, 2023 at 08:46:56AM +0900, Youngmin Nam wrote:
> > On Wed, Apr 26, 2023 at 02:06:25PM +0900, Youngmin Nam wrote:
> > > On Tue, Apr 25, 2023 at 02:39:51PM +0100, Mark Rutland wrote:
> > > > On Tue, Apr 25, 2023 at 11:31:31AM +0900, Youngmin Nam wrote:
> > > > > On Mon, Apr 24, 2023 at 02:08:14PM +0100, Mark Rutland wrote:
> > > > > > With that in mind, I think what we should do is cut this at the instant we
> > > > > > enter the exception; for the trace below that would be el1h_64_irq. I've added
> > > > > > some line spacing there to make it stand out.
> 
> > > > I'd meant something like the below, marking the assembly (as x86 does) rather
> > > > than the C code. I'll try to sort that out and send a proper patch series after
> > > > -rc1.
> > > > 
> > > > Thanks,
> > > > Mark.
> > 
> > Hi Mark.
> > This is gentle remind for you.
> > Can I know that you've sent the patch ?
> > Actually I'm looking forward to seeing your patch. :)
> 
> Sorry; I haven't yet sent this out as I'm still looking into how this interacts
> with ftrace.
> 
> I'll try to flesh out the commit message and get this out next week. You will
> be Cc'd when I send it out.
> 
> Thanks,
> Mark.
> 
Hi Mark.
Sorry to rush you. Would you share your patch for us ? We're still waiting
your patch. :)

Thanks.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24  1:04 ` [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Youngmin Nam
  2023-04-24  6:48   ` Dmitry Vyukov
  2023-04-24 11:01   ` Mark Rutland
@ 2023-06-08  9:03   ` Mark Rutland
  2023-06-08 18:17   ` Catalin Marinas
  3 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2023-06-08  9:03 UTC (permalink / raw)
  To: Youngmin Nam, catalin.marinas
  Cc: will, anshuman.khandual, broonie, alexandru.elisei, ardb,
	linux-arm-kernel, hy50.seo, dvyukov, andreyknvl

Hi,

Sorry for the long radiosilence -- when looking into the alternative approach I
suggested there are a number of other issues in this area that will tickle.
While I'd still like us to head in that direction, for that to work we'll need
to do some larger (kernel-wide) cleanup, and I don't think that should hold up
improving the situation for KASAN and friends.

Given that, I think we should take this patch for now, so FWIW:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Catalin, are you happy to pick this up?

The patch itself is safe as:

(a) On arm64, kprobes blacklists the __irqentry_text_start..__irqentry_text_end
    region, so this doesn't result in a functional change to the blacklisting.
 
(b) The functions marked with __irq_entry aren't used within kprobes anyway, so
    would be safe to kprobe regardless, but we can leave that to a separate
    patch.

The only reason we have the __kprobes annotation in the
!CONFIG_FUNCTION_GRAPH_TRACER case is that we used to have an __exception
annotation which covered this and the actual entry assembly, and when that got
removed in commit:

  b6e43c0e3129ffe8 ("arm64: remove __exception annotations")

... we replaced that with __kprobes out of an abundance of caution.

Thanks,
Mark.

On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote:
> filter_irq_stacks() is supposed to cut entries which are related irq entries
> from its call stack.
> And in_irqentry_text() which is called by filter_irq_stacks()
> uses __irqentry_text_start/end symbol to find irq entries in callstack.
> 
> But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> 
> This problem can makes unintentional deep call stack entries especially
> in KASAN enabled situation as below.
> 
> [ 2479.383395]I[0:launcher-loader: 1719] Stack depot reached limit capacity
> [ 2479.383538]I[0:launcher-loader: 1719] WARNING: CPU: 0 PID: 1719 at lib/stackdepot.c:129 __stack_depot_save+0x464/0x46c
> [ 2479.385693]I[0:launcher-loader: 1719] pstate: 624000c5 (nZCv daIF +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> [ 2479.385724]I[0:launcher-loader: 1719] pc : __stack_depot_save+0x464/0x46c
> [ 2479.385751]I[0:launcher-loader: 1719] lr : __stack_depot_save+0x460/0x46c
> [ 2479.385774]I[0:launcher-loader: 1719] sp : ffffffc0080073c0
> [ 2479.385793]I[0:launcher-loader: 1719] x29: ffffffc0080073e0 x28: ffffffd00b78a000 x27: 0000000000000000
> [ 2479.385839]I[0:launcher-loader: 1719] x26: 000000000004d1dd x25: ffffff891474f000 x24: 00000000ca64d1dd
> [ 2479.385882]I[0:launcher-loader: 1719] x23: 0000000000000200 x22: 0000000000000220 x21: 0000000000000040
> [ 2479.385925]I[0:launcher-loader: 1719] x20: ffffffc008007440 x19: 0000000000000000 x18: 0000000000000000
> [ 2479.385969]I[0:launcher-loader: 1719] x17: 2065726568207475 x16: 000000000000005e x15: 2d2d2d2d2d2d2d20
> [ 2479.386013]I[0:launcher-loader: 1719] x14: 5d39313731203a72 x13: 00000000002f6b30 x12: 00000000002f6af8
> [ 2479.386057]I[0:launcher-loader: 1719] x11: 00000000ffffffff x10: ffffffb90aacf000 x9 : e8a74a6c16008800
> [ 2479.386101]I[0:launcher-loader: 1719] x8 : e8a74a6c16008800 x7 : 00000000002f6b30 x6 : 00000000002f6af8
> [ 2479.386145]I[0:launcher-loader: 1719] x5 : ffffffc0080070c8 x4 : ffffffd00b192380 x3 : ffffffd0092b313c
> [ 2479.386189]I[0:launcher-loader: 1719] x2 : 0000000000000001 x1 : 0000000000000004 x0 : 0000000000000022
> [ 2479.386231]I[0:launcher-loader: 1719] Call trace:
> [ 2479.386248]I[0:launcher-loader: 1719]  __stack_depot_save+0x464/0x46c
> [ 2479.386273]I[0:launcher-loader: 1719]  kasan_save_stack+0x58/0x70
> [ 2479.386303]I[0:launcher-loader: 1719]  save_stack_info+0x34/0x138
> [ 2479.386331]I[0:launcher-loader: 1719]  kasan_save_free_info+0x18/0x24
> [ 2479.386358]I[0:launcher-loader: 1719]  ____kasan_slab_free+0x16c/0x170
> [ 2479.386385]I[0:launcher-loader: 1719]  __kasan_slab_free+0x10/0x20
> [ 2479.386410]I[0:launcher-loader: 1719]  kmem_cache_free+0x238/0x53c
> [ 2479.386435]I[0:launcher-loader: 1719]  mempool_free_slab+0x1c/0x28
> [ 2479.386460]I[0:launcher-loader: 1719]  mempool_free+0x7c/0x1a0
> [ 2479.386484]I[0:launcher-loader: 1719]  bvec_free+0x34/0x80
> [ 2479.386514]I[0:launcher-loader: 1719]  bio_free+0x60/0x98
> [ 2479.386540]I[0:launcher-loader: 1719]  bio_put+0x50/0x21c
> [ 2479.386567]I[0:launcher-loader: 1719]  f2fs_write_end_io+0x4ac/0x4d0
> [ 2479.386594]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386622]I[0:launcher-loader: 1719]  __dm_io_complete+0x324/0x37c
> [ 2479.386650]I[0:launcher-loader: 1719]  dm_io_dec_pending+0x60/0xa4
> [ 2479.386676]I[0:launcher-loader: 1719]  clone_endio+0xf8/0x2f0
> [ 2479.386700]I[0:launcher-loader: 1719]  bio_endio+0x2dc/0x300
> [ 2479.386727]I[0:launcher-loader: 1719]  blk_update_request+0x258/0x63c
> [ 2479.386754]I[0:launcher-loader: 1719]  scsi_end_request+0x50/0x304
> [ 2479.386782]I[0:launcher-loader: 1719]  scsi_io_completion+0x88/0x160
> [ 2479.386808]I[0:launcher-loader: 1719]  scsi_finish_command+0x17c/0x194
> [ 2479.386833]I[0:launcher-loader: 1719]  scsi_complete+0xcc/0x158
> [ 2479.386859]I[0:launcher-loader: 1719]  blk_mq_complete_request+0x4c/0x5c
> [ 2479.386885]I[0:launcher-loader: 1719]  scsi_done_internal+0xf4/0x1e0
> [ 2479.386910]I[0:launcher-loader: 1719]  scsi_done+0x14/0x20
> [ 2479.386935]I[0:launcher-loader: 1719]  ufshcd_compl_one_cqe+0x578/0x71c
> [ 2479.386963]I[0:launcher-loader: 1719]  ufshcd_mcq_poll_cqe_nolock+0xc8/0x150
> [ 2479.386991]I[0:launcher-loader: 1719]  ufshcd_intr+0x868/0xc0c
> [ 2479.387017]I[0:launcher-loader: 1719]  __handle_irq_event_percpu+0xd0/0x348
> [ 2479.387044]I[0:launcher-loader: 1719]  handle_irq_event_percpu+0x24/0x74
> [ 2479.387068]I[0:launcher-loader: 1719]  handle_irq_event+0x74/0xe0
> [ 2479.387091]I[0:launcher-loader: 1719]  handle_fasteoi_irq+0x174/0x240
> [ 2479.387118]I[0:launcher-loader: 1719]  handle_irq_desc+0x7c/0x2c0
> [ 2479.387147]I[0:launcher-loader: 1719]  generic_handle_domain_irq+0x1c/0x28
> [ 2479.387174]I[0:launcher-loader: 1719]  gic_handle_irq+0x64/0x158
> [ 2479.387204]I[0:launcher-loader: 1719]  call_on_irq_stack+0x2c/0x54
> [ 2479.387231]I[0:launcher-loader: 1719]  do_interrupt_handler+0x70/0xa0
> [ 2479.387258]I[0:launcher-loader: 1719]  el1_interrupt+0x34/0x68
> [ 2479.387283]I[0:launcher-loader: 1719]  el1h_64_irq_handler+0x18/0x24
> [ 2479.387308]I[0:launcher-loader: 1719]  el1h_64_irq+0x68/0x6c
> [ 2479.387332]I[0:launcher-loader: 1719]  blk_attempt_bio_merge+0x8/0x170
> [ 2479.387356]I[0:launcher-loader: 1719]  blk_mq_attempt_bio_merge+0x78/0x98
> [ 2479.387383]I[0:launcher-loader: 1719]  blk_mq_submit_bio+0x324/0xa40
> [ 2479.387409]I[0:launcher-loader: 1719]  __submit_bio+0x104/0x138
> [ 2479.387436]I[0:launcher-loader: 1719]  submit_bio_noacct_nocheck+0x1d0/0x4a0
> [ 2479.387462]I[0:launcher-loader: 1719]  submit_bio_noacct+0x618/0x804
> [ 2479.387487]I[0:launcher-loader: 1719]  submit_bio+0x164/0x180
> [ 2479.387511]I[0:launcher-loader: 1719]  f2fs_submit_read_bio+0xe4/0x1c4
> [ 2479.387537]I[0:launcher-loader: 1719]  f2fs_mpage_readpages+0x888/0xa4c
> [ 2479.387563]I[0:launcher-loader: 1719]  f2fs_readahead+0xd4/0x19c
> [ 2479.387587]I[0:launcher-loader: 1719]  read_pages+0xb0/0x4ac
> [ 2479.387614]I[0:launcher-loader: 1719]  page_cache_ra_unbounded+0x238/0x288
> [ 2479.387642]I[0:launcher-loader: 1719]  do_page_cache_ra+0x60/0x6c
> [ 2479.387669]I[0:launcher-loader: 1719]  page_cache_ra_order+0x318/0x364
> [ 2479.387695]I[0:launcher-loader: 1719]  ondemand_readahead+0x30c/0x3d8
> [ 2479.387722]I[0:launcher-loader: 1719]  page_cache_sync_ra+0xb4/0xc8
> [ 2479.387749]I[0:launcher-loader: 1719]  filemap_read+0x268/0xd24
> [ 2479.387777]I[0:launcher-loader: 1719]  f2fs_file_read_iter+0x1a0/0x62c
> [ 2479.387806]I[0:launcher-loader: 1719]  vfs_read+0x258/0x34c
> [ 2479.387831]I[0:launcher-loader: 1719]  ksys_pread64+0x8c/0xd0
> [ 2479.387857]I[0:launcher-loader: 1719]  __arm64_sys_pread64+0x48/0x54
> [ 2479.387881]I[0:launcher-loader: 1719]  invoke_syscall+0x58/0x158
> [ 2479.387909]I[0:launcher-loader: 1719]  el0_svc_common+0xf0/0x134
> [ 2479.387935]I[0:launcher-loader: 1719]  do_el0_svc+0x44/0x114
> [ 2479.387961]I[0:launcher-loader: 1719]  el0_svc+0x2c/0x80
> [ 2479.387985]I[0:launcher-loader: 1719]  el0t_64_sync_handler+0x48/0x114
> [ 2479.388010]I[0:launcher-loader: 1719]  el0t_64_sync+0x190/0x194
> [ 2479.388038]I[0:launcher-loader: 1719] Kernel panic - not syncing: kernel: panic_on_warn set ...
> 
> So let's set __exception_irq_entry with __irq_entry as a default.
> Applying this patch, we can see gic_hande_irq is included in Systemp.map as below.
> 
> * Before
> ffffffc008010000 T __do_softirq
> ffffffc008010000 T __irqentry_text_end
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T __softirqentry_text_start
> ffffffc008010000 T _stext
> ffffffc00801066c T __softirqentry_text_end
> ffffffc008010670 T __entry_text_start
> 
> * After
> ffffffc008010000 T __irqentry_text_start
> ffffffc008010000 T _stext
> ffffffc008010000 t gic_handle_irq
> ffffffc00801013c t gic_handle_irq
> ffffffc008010294 T __irqentry_text_end
> ffffffc008010298 T __do_softirq
> ffffffc008010298 T __softirqentry_text_start
> ffffffc008010904 T __softirqentry_text_end
> ffffffc008010908 T __entry_text_start
> 
> Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> Change-Id: Iea7ff528be1c72cf50ab6aabafa77215ddb55eb2
> ---
>  arch/arm64/include/asm/exception.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index 19713d0f013b..18dbb35a337f 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -8,16 +8,11 @@
>  #define __ASM_EXCEPTION_H
>  
>  #include <asm/esr.h>
> -#include <asm/kprobes.h>
>  #include <asm/ptrace.h>
>  
>  #include <linux/interrupt.h>
>  
> -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  #define __exception_irq_entry	__irq_entry
> -#else
> -#define __exception_irq_entry	__kprobes
> -#endif
>  
>  static inline unsigned long disr_to_esr(u64 disr)
>  {
> -- 
> 2.39.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
  2023-04-24  1:04 ` [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Youngmin Nam
                     ` (2 preceding siblings ...)
  2023-06-08  9:03   ` Mark Rutland
@ 2023-06-08 18:17   ` Catalin Marinas
  3 siblings, 0 replies; 11+ messages in thread
From: Catalin Marinas @ 2023-06-08 18:17 UTC (permalink / raw)
  To: will, Youngmin Nam
  Cc: mark.rutland, anshuman.khandual, broonie, alexandru.elisei, ardb,
	linux-arm-kernel, hy50.seo, dvyukov, andreyknvl

On Mon, 24 Apr 2023 10:04:36 +0900, Youngmin Nam wrote:
> filter_irq_stacks() is supposed to cut entries which are related irq entries
> from its call stack.
> And in_irqentry_text() which is called by filter_irq_stacks()
> uses __irqentry_text_start/end symbol to find irq entries in callstack.
> 
> But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER",
> arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq
> between __irqentry_text_start and __irqentry_text_end as we discussed in below link.
> https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: set __exception_irq_entry with __irq_entry as a default
      https://git.kernel.org/arm64/c/f6794950f0e5

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-06-08 18:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20230424003252epcas2p29758e056b4766e53c252b5927a0cb406@epcas2p2.samsung.com>
2023-04-24  1:04 ` [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Youngmin Nam
2023-04-24  6:48   ` Dmitry Vyukov
2023-04-24 11:01   ` Mark Rutland
2023-04-24 12:09     ` Dmitry Vyukov
2023-04-24 13:08       ` Mark Rutland
2023-04-25  2:31         ` Youngmin Nam
2023-04-25 13:39           ` Mark Rutland
2023-04-26  5:06             ` Youngmin Nam
     [not found]               ` <ZF1+cLp7Io7L25yG@perf>
     [not found]                 ` <ZF5gmBz4NbDseDHp@FVFF77S0Q05N>
2023-05-25 23:20                   ` Youngmin Nam
2023-06-08  9:03   ` Mark Rutland
2023-06-08 18:17   ` Catalin Marinas

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.