* Re: [PATCHv2] wifi: ath11k: mark reset srng lists as uninitialized [not found] ` <rxaibpgtf37w3th5lnt3w574w443yononqjoglyfktlcxe26qg@jpmtkckrotzp> @ 2025-06-12 8:01 ` Baochen Qiang 2025-06-12 10:01 ` Pedro Falcato 0 siblings, 1 reply; 3+ messages in thread From: Baochen Qiang @ 2025-06-12 8:01 UTC (permalink / raw) To: Sergey Senozhatsky, akpm Cc: Jeff Johnson, linux-wireless, ath11k, linux-kernel, linux-mm [+ kernel mm list] On 6/12/2025 1:04 PM, Sergey Senozhatsky wrote: > On (25/06/12 11:30), Baochen Qiang wrote: >> On 5/29/2025 11:56 AM, Sergey Senozhatsky wrote: >>> ath11k_hal_srng_deinit() frees rdp and wrp which are used >>> by srng lists. Mark srng lists as not-initialized. This >>> makes sense, for instance, when device fails to resume >>> and the driver calls ath11k_hal_srng_deinit() from >>> ath11k_core_reconfigure_on_crash(). >> >> Did you see any issue without your change? > > We do see some issues, yes, on LTS kernels. > > [..] >>> diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c >>> index 8cb1505a5a0c..cab11a35f911 100644 >>> --- a/drivers/net/wireless/ath/ath11k/hal.c >>> +++ b/drivers/net/wireless/ath/ath11k/hal.c >>> @@ -1346,6 +1346,10 @@ EXPORT_SYMBOL(ath11k_hal_srng_init); >>> void ath11k_hal_srng_deinit(struct ath11k_base *ab) >>> { >>> struct ath11k_hal *hal = &ab->hal; >>> + int i; >>> + >>> + for (i = 0; i < HAL_SRNG_RING_ID_MAX; i++) >>> + ab->hal.srng_list[i].initialized = 0; >> >> With this flag reset, srng stats would not be dumped in ath11k_hal_dump_srng_stats(). > > I think un-initialized lists should not be dumped. > > ath11k_hal_srng_deinit() releases wrp.vaddr and rdp.vaddr, which are > accessed, as far as I understand it, in ath11k_hal_dump_srng_stats() > as *srng->u.src_ring.tp_addr and *srng->u.dst_ring.hp_addr, presumably, > causing things like: > > <1>[173154.396775] BUG: unable to handle page fault for address: ffffb4e4c046f010 > <1>[173154.396778] #PF: supervisor read access in kernel mode > <1>[173154.396781] #PF: error_code(0x0000) - not-present page I am confused here: if the root cause is driver trying to read a freed memory, it should not result in a PF issue. Because even if freed, the page is there and still mapped in kernel page table. Andrew, any insights? > <4>[173154.396824] RIP: 0010:ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k] > <4>[173154.396839] Code: 88 c0 44 89 f2 89 c1 e8 3a 14 06 00 41 be e8 25 00 00 eb 6e 42 0f b6 84 33 78 ff ff ff 89 45 d0 46 8b 7c 33 d8 4a 8b 44 33 e0 <44> 8b 20 46 8b 6c 33 e8 42 8b 04 33 48 89 45 c8 48 8b 3d 45 a3 a0 > <4>[173154.396842] RSP: 0018:ffffb4e4dceefc50 EFLAGS: 00010246 > <4>[173154.396846] RAX: ffffb4e4c046f010 RBX: ffff90d1c3040000 RCX: a0009634a5d28c00 > <4>[173154.396849] RDX: ffffffffb0279d80 RSI: ffffffffb0279d80 RDI: ffff90d2e5d17488 > <4>[173154.396851] RBP: ffffb4e4dceefc90 R08: ffffffffb0249d80 R09: 0000000000003b82 > <4>[173154.396854] R10: 0000000000000004 R11: 00000000ffffffea R12: ffff90d1c3041c90 > <4>[173154.396856] R13: ffff90d1c3040000 R14: 0000000000002828 R15: 0000000000000000 > <4>[173154.396859] FS: 0000000000000000(0000) GS:ffff90d2e5d00000(0000) knlGS:0000000000000000 > <4>[173154.396862] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > <4>[173154.396865] CR2: ffffb4e4c046f010 CR3: 000000005ca24000 CR4: 0000000000750ee0 > <4>[173154.396868] PKRU: 55555554 > <4>[173154.396870] Call Trace: > <4>[173154.396874] <TASK> > <4>[173154.396883] ? __die_body+0xae/0xb0 > <4>[173154.396890] ? page_fault_oops+0x381/0x3e0 > <4>[173154.396896] ? exc_page_fault+0x69/0xa0 > <4>[173154.396901] ? asm_exc_page_fault+0x22/0x30 > <4>[173154.396908] ? ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k (HASH:3de7 4)] > <4>[173154.396923] ath11k_qmi_driver_event_work+0xbd/0x1050 [ath11k (HASH:3de7 4)] > <4>[173154.396942] worker_thread+0x390/0x960 > <4>[173154.396949] kthread+0x149/0x170 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] wifi: ath11k: mark reset srng lists as uninitialized 2025-06-12 8:01 ` [PATCHv2] wifi: ath11k: mark reset srng lists as uninitialized Baochen Qiang @ 2025-06-12 10:01 ` Pedro Falcato 2025-06-12 10:23 ` Baochen Qiang 0 siblings, 1 reply; 3+ messages in thread From: Pedro Falcato @ 2025-06-12 10:01 UTC (permalink / raw) To: Baochen Qiang Cc: Sergey Senozhatsky, akpm, Jeff Johnson, linux-wireless, ath11k, linux-kernel, linux-mm On Thu, Jun 12, 2025 at 04:01:49PM +0800, Baochen Qiang wrote: > [+ kernel mm list] > > On 6/12/2025 1:04 PM, Sergey Senozhatsky wrote: > > On (25/06/12 11:30), Baochen Qiang wrote: > >> On 5/29/2025 11:56 AM, Sergey Senozhatsky wrote: > >>> ath11k_hal_srng_deinit() frees rdp and wrp which are used > >>> by srng lists. Mark srng lists as not-initialized. This > >>> makes sense, for instance, when device fails to resume > >>> and the driver calls ath11k_hal_srng_deinit() from > >>> ath11k_core_reconfigure_on_crash(). > >> > >> Did you see any issue without your change? > > > > We do see some issues, yes, on LTS kernels. > > > > [..] > >>> diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c > >>> index 8cb1505a5a0c..cab11a35f911 100644 > >>> --- a/drivers/net/wireless/ath/ath11k/hal.c > >>> +++ b/drivers/net/wireless/ath/ath11k/hal.c > >>> @@ -1346,6 +1346,10 @@ EXPORT_SYMBOL(ath11k_hal_srng_init); > >>> void ath11k_hal_srng_deinit(struct ath11k_base *ab) > >>> { > >>> struct ath11k_hal *hal = &ab->hal; > >>> + int i; > >>> + > >>> + for (i = 0; i < HAL_SRNG_RING_ID_MAX; i++) > >>> + ab->hal.srng_list[i].initialized = 0; > >> > >> With this flag reset, srng stats would not be dumped in ath11k_hal_dump_srng_stats(). > > > > I think un-initialized lists should not be dumped. > > > > ath11k_hal_srng_deinit() releases wrp.vaddr and rdp.vaddr, which are > > accessed, as far as I understand it, in ath11k_hal_dump_srng_stats() > > as *srng->u.src_ring.tp_addr and *srng->u.dst_ring.hp_addr, presumably, > > causing things like: > > > > <1>[173154.396775] BUG: unable to handle page fault for address: ffffb4e4c046f010 > > <1>[173154.396778] #PF: supervisor read access in kernel mode > > <1>[173154.396781] #PF: error_code(0x0000) - not-present page > > I am confused here: if the root cause is driver trying to read a freed memory, it should > not result in a PF issue. Because even if freed, the page is there and still mapped in > kernel page table. > Any memory that is virtually-mapped (read: vmalloc, vmap, vm_map_ram, and others) will be unmapped on its subsequent free. I'm not familiar with the DMA subsystem, but the address ffffb4e4c046f010 is vmalloc-like. > > > <4>[173154.396824] RIP: 0010:ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k] > > <4>[173154.396839] Code: 88 c0 44 89 f2 89 c1 e8 3a 14 06 00 41 be e8 25 00 00 eb 6e 42 0f b6 84 33 78 ff ff ff 89 45 d0 46 8b 7c 33 d8 4a 8b 44 33 e0 <44> 8b 20 46 8b 6c 33 e8 42 8b 04 33 48 89 45 c8 48 8b 3d 45 a3 a0 > > <4>[173154.396842] RSP: 0018:ffffb4e4dceefc50 EFLAGS: 00010246 > > <4>[173154.396846] RAX: ffffb4e4c046f010 RBX: ffff90d1c3040000 RCX: a0009634a5d28c00 > > <4>[173154.396849] RDX: ffffffffb0279d80 RSI: ffffffffb0279d80 RDI: ffff90d2e5d17488 > > <4>[173154.396851] RBP: ffffb4e4dceefc90 R08: ffffffffb0249d80 R09: 0000000000003b82 > > <4>[173154.396854] R10: 0000000000000004 R11: 00000000ffffffea R12: ffff90d1c3041c90 > > <4>[173154.396856] R13: ffff90d1c3040000 R14: 0000000000002828 R15: 0000000000000000 > > <4>[173154.396859] FS: 0000000000000000(0000) GS:ffff90d2e5d00000(0000) knlGS:0000000000000000 > > <4>[173154.396862] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > <4>[173154.396865] CR2: ffffb4e4c046f010 CR3: 000000005ca24000 CR4: 0000000000750ee0 > > <4>[173154.396868] PKRU: 55555554 > > <4>[173154.396870] Call Trace: > > <4>[173154.396874] <TASK> > > <4>[173154.396883] ? __die_body+0xae/0xb0 > > <4>[173154.396890] ? page_fault_oops+0x381/0x3e0 > > <4>[173154.396896] ? exc_page_fault+0x69/0xa0 > > <4>[173154.396901] ? asm_exc_page_fault+0x22/0x30 > > <4>[173154.396908] ? ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k (HASH:3de7 4)] > > <4>[173154.396923] ath11k_qmi_driver_event_work+0xbd/0x1050 [ath11k (HASH:3de7 4)] > > <4>[173154.396942] worker_thread+0x390/0x960 > > <4>[173154.396949] kthread+0x149/0x170 > -- Pedro ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] wifi: ath11k: mark reset srng lists as uninitialized 2025-06-12 10:01 ` Pedro Falcato @ 2025-06-12 10:23 ` Baochen Qiang 0 siblings, 0 replies; 3+ messages in thread From: Baochen Qiang @ 2025-06-12 10:23 UTC (permalink / raw) To: Pedro Falcato Cc: Sergey Senozhatsky, akpm, Jeff Johnson, linux-wireless, ath11k, linux-kernel, linux-mm On 6/12/2025 6:01 PM, Pedro Falcato wrote: > On Thu, Jun 12, 2025 at 04:01:49PM +0800, Baochen Qiang wrote: >> [+ kernel mm list] >> >> On 6/12/2025 1:04 PM, Sergey Senozhatsky wrote: >>> On (25/06/12 11:30), Baochen Qiang wrote: >>>> On 5/29/2025 11:56 AM, Sergey Senozhatsky wrote: >>>>> ath11k_hal_srng_deinit() frees rdp and wrp which are used >>>>> by srng lists. Mark srng lists as not-initialized. This >>>>> makes sense, for instance, when device fails to resume >>>>> and the driver calls ath11k_hal_srng_deinit() from >>>>> ath11k_core_reconfigure_on_crash(). >>>> >>>> Did you see any issue without your change? >>> >>> We do see some issues, yes, on LTS kernels. >>> >>> [..] >>>>> diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c >>>>> index 8cb1505a5a0c..cab11a35f911 100644 >>>>> --- a/drivers/net/wireless/ath/ath11k/hal.c >>>>> +++ b/drivers/net/wireless/ath/ath11k/hal.c >>>>> @@ -1346,6 +1346,10 @@ EXPORT_SYMBOL(ath11k_hal_srng_init); >>>>> void ath11k_hal_srng_deinit(struct ath11k_base *ab) >>>>> { >>>>> struct ath11k_hal *hal = &ab->hal; >>>>> + int i; >>>>> + >>>>> + for (i = 0; i < HAL_SRNG_RING_ID_MAX; i++) >>>>> + ab->hal.srng_list[i].initialized = 0; >>>> >>>> With this flag reset, srng stats would not be dumped in ath11k_hal_dump_srng_stats(). >>> >>> I think un-initialized lists should not be dumped. >>> >>> ath11k_hal_srng_deinit() releases wrp.vaddr and rdp.vaddr, which are >>> accessed, as far as I understand it, in ath11k_hal_dump_srng_stats() >>> as *srng->u.src_ring.tp_addr and *srng->u.dst_ring.hp_addr, presumably, >>> causing things like: >>> >>> <1>[173154.396775] BUG: unable to handle page fault for address: ffffb4e4c046f010 >>> <1>[173154.396778] #PF: supervisor read access in kernel mode >>> <1>[173154.396781] #PF: error_code(0x0000) - not-present page >> >> I am confused here: if the root cause is driver trying to read a freed memory, it should >> not result in a PF issue. Because even if freed, the page is there and still mapped in >> kernel page table. >> > > Any memory that is virtually-mapped (read: vmalloc, vmap, vm_map_ram, and others) > will be unmapped on its subsequent free. I'm not familiar with the DMA subsystem, > but the address ffffb4e4c046f010 is vmalloc-like. OK, I forget the vmalloc case. And indeed in case IOMMU present, the DMA subsystem is preferring vmalloc'ed memory. Thank you, Pedro! > >> >>> <4>[173154.396824] RIP: 0010:ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k] >>> <4>[173154.396839] Code: 88 c0 44 89 f2 89 c1 e8 3a 14 06 00 41 be e8 25 00 00 eb 6e 42 0f b6 84 33 78 ff ff ff 89 45 d0 46 8b 7c 33 d8 4a 8b 44 33 e0 <44> 8b 20 46 8b 6c 33 e8 42 8b 04 33 48 89 45 c8 48 8b 3d 45 a3 a0 >>> <4>[173154.396842] RSP: 0018:ffffb4e4dceefc50 EFLAGS: 00010246 >>> <4>[173154.396846] RAX: ffffb4e4c046f010 RBX: ffff90d1c3040000 RCX: a0009634a5d28c00 >>> <4>[173154.396849] RDX: ffffffffb0279d80 RSI: ffffffffb0279d80 RDI: ffff90d2e5d17488 >>> <4>[173154.396851] RBP: ffffb4e4dceefc90 R08: ffffffffb0249d80 R09: 0000000000003b82 >>> <4>[173154.396854] R10: 0000000000000004 R11: 00000000ffffffea R12: ffff90d1c3041c90 >>> <4>[173154.396856] R13: ffff90d1c3040000 R14: 0000000000002828 R15: 0000000000000000 >>> <4>[173154.396859] FS: 0000000000000000(0000) GS:ffff90d2e5d00000(0000) knlGS:0000000000000000 >>> <4>[173154.396862] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> <4>[173154.396865] CR2: ffffb4e4c046f010 CR3: 000000005ca24000 CR4: 0000000000750ee0 >>> <4>[173154.396868] PKRU: 55555554 >>> <4>[173154.396870] Call Trace: >>> <4>[173154.396874] <TASK> >>> <4>[173154.396883] ? __die_body+0xae/0xb0 >>> <4>[173154.396890] ? page_fault_oops+0x381/0x3e0 >>> <4>[173154.396896] ? exc_page_fault+0x69/0xa0 >>> <4>[173154.396901] ? asm_exc_page_fault+0x22/0x30 >>> <4>[173154.396908] ? ath11k_hal_dump_srng_stats+0x2b4/0x3b0 [ath11k (HASH:3de7 4)] >>> <4>[173154.396923] ath11k_qmi_driver_event_work+0xbd/0x1050 [ath11k (HASH:3de7 4)] >>> <4>[173154.396942] worker_thread+0x390/0x960 >>> <4>[173154.396949] kthread+0x149/0x170 >> > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-12 10:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20250529035708.3136232-1-senozhatsky@chromium.org>
[not found] ` <5857380e-ced9-4118-b9a0-ce926b25c11c@quicinc.com>
[not found] ` <rxaibpgtf37w3th5lnt3w574w443yononqjoglyfktlcxe26qg@jpmtkckrotzp>
2025-06-12 8:01 ` [PATCHv2] wifi: ath11k: mark reset srng lists as uninitialized Baochen Qiang
2025-06-12 10:01 ` Pedro Falcato
2025-06-12 10:23 ` Baochen Qiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox