[PATCH] arm64: set __exception_irq_entry with __irq_entry as a default
Youngmin Nam
youngmin.nam at samsung.com
Thu May 11 16:46:56 PDT 2023
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:
> > > > 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 at 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.
> >
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. :)
> 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----
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list