Mailing List Archive: [PATCH] firewire: core: obsolete usage of GFP

[PATCH] firewire: core: obsolete usage of GFP_ATOMIC at building node tree

Jun 4, 2023, 12:15 AM

Post #1 of 4 (10 views)

The flag of GFP_ATOMIC is given to the call of kmalloc when building node
tree, but the call is not atomic context. The call of
fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree,
while they are done in specific workqueue or pci remove callback.

This commit obsolete the usage of GFP_ATOMIC.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
drivers/firewire/core-device.c | 2 +-
drivers/firewire/core-topology.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
index aa597cda0d88..a3104e35412c 100644
--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -1211,7 +1211,7 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event)
* without actually having a link.
*/
create:
- device = kzalloc(sizeof(*device), GFP_ATOMIC);
+ device = kzalloc(sizeof(*device), GFP_KERNEL);
if (device == NULL)
break;

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index f40c81534381..88466b663482 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -101,7 +101,7 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color)
{
struct fw_node *node;

- node = kzalloc(struct_size(node, ports, port_count), GFP_ATOMIC);
+ node = kzalloc(struct_size(node, ports, port_count), GFP_KERNEL);
if (node == NULL)
return NULL;

--
2.39.2

Re: [PATCH] firewire: core: obsolete usage of GFP_ATOMIC at building node tree [ In reply to ]

o-takashi at sakamocchi

Jun 5, 2023, 4:15 PM

Post #2 of 4 (10 views)

Permalink

On Sun, Jun 04, 2023 at 04:02:55PM +0900, Takashi Sakamoto wrote:
> The flag of GFP_ATOMIC is given to the call of kmalloc when building node
> tree, but the call is not atomic context. The call of
> fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree,
> while they are done in specific workqueue or pci remove callback.
>
> This commit obsolete the usage of GFP_ATOMIC.
>
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
> drivers/firewire/core-device.c | 2 +-
> drivers/firewire/core-topology.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)

Applied to for-next branch.

Regards

Takashi Sakamoto

Re: [PATCH] firewire: core: obsolete usage of GFP_ATOMIC at building node tree [ In reply to ]

john.ogness at linutronix

Sep 13, 2023, 4:15 AM

Post #3 of 4 (10 views)

Permalink

On 2023-06-04, Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:
> The flag of GFP_ATOMIC is given to the call of kmalloc when building node
> tree, but the call is not atomic context. The call of
> fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree,
> while they are done in specific workqueue or pci remove callback.

The comment is incorrect about not being in atomic
context. fw_core_handle_bus_reset() calls build_tree() with the
card->lock spinlock held. With 6.6-rc1 I see:

[ 12.555784] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306
[ 12.555808] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 70, name: kworker/1:2
[ 12.555814] preempt_count: 1, expected: 0
[ 12.555820] INFO: lockdep is turned off.
[ 12.555824] irq event stamp: 208
[ 12.555828] hardirqs last enabled at (207): [<c00000000111e414>] ._raw_spin_unlock_irq+0x44/0x80
[ 12.555850] hardirqs last disabled at (208): [<c00000000110ff94>] .__schedule+0x854/0xfe0
[ 12.555859] softirqs last enabled at (188): [<c000000000f73504>] .addrconf_verify_rtnl+0x2c4/0xb70
[ 12.555872] softirqs last disabled at (182): [<c000000000f732b0>] .addrconf_verify_rtnl+0x70/0xb70
[ 12.555884] CPU: 1 PID: 70 Comm: kworker/1:2 Tainted: G S 6.6.0-rc1 #1
[ 12.555893] Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac
[ 12.555898] Workqueue: firewire_ohci .bus_reset_work [firewire_ohci]
[ 12.555939] Call Trace:
[ 12.555944] [c000000009677830] [c0000000010d83c0] .dump_stack_lvl+0x8c/0xd0 (unreliable)
[ 12.555963] [c0000000096778b0] [c000000000140270] .__might_resched+0x320/0x340
[ 12.555978] [c000000009677940] [c000000000497600] .__kmem_cache_alloc_node+0x390/0x460
[ 12.555993] [c000000009677a10] [c0000000003fe620] .__kmalloc+0x70/0x310
[ 12.556007] [c000000009677ac0] [c0003d00004e2268] .fw_core_handle_bus_reset+0x2c8/0xba0 [firewire_core]
[ 12.556060] [c000000009677c20] [c0003d0000491190] .bus_reset_work+0x330/0x9b0 [firewire_ohci]
[ 12.556079] [c000000009677d10] [c00000000011d0d0] .process_one_work+0x280/0x6f0
[ 12.556094] [c000000009677e10] [c00000000011d8a0] .worker_thread+0x360/0x500
[ 12.556107] [c000000009677ef0] [c00000000012e3b4] .kthread+0x154/0x160
[ 12.556120] [c000000009677f90] [c00000000000bfa8] .start_kernel_thread+0x10/0x14

John Ogness

Re: [PATCH] firewire: core: obsolete usage of GFP_ATOMIC at building node tree [ In reply to ]

o-takashi at sakamocchi

Sep 15, 2023, 4:15 AM

Post #4 of 4 (10 views)

Permalink

Hi,

On Wed, Sep 13, 2023 at 11:54:39AM +0206, John Ogness wrote:
> On 2023-06-04, Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:
> > The flag of GFP_ATOMIC is given to the call of kmalloc when building node
> > tree, but the call is not atomic context. The call of
> > fw_core_handle_bus_reset() and fw_core_remove_card() builds the tree,
> > while they are done in specific workqueue or pci remove callback.
>
> The comment is incorrect about not being in atomic
> context. fw_core_handle_bus_reset() calls build_tree() with the
> card->lock spinlock held. With 6.6-rc1 I see:

Thanks for your report. Just now I sent fixes to Linus for 6.6-rc1. It is
expected to apply it to stable 6.5 kernel as well.

> [ 12.555784] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306
> [ 12.555808] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 70, name: kworker/1:2
> [ 12.555814] preempt_count: 1, expected: 0
> [ 12.555820] INFO: lockdep is turned off.
> [ 12.555824] irq event stamp: 208
> [ 12.555828] hardirqs last enabled at (207): [<c00000000111e414>] ._raw_spin_unlock_irq+0x44/0x80
> [ 12.555850] hardirqs last disabled at (208): [<c00000000110ff94>] .__schedule+0x854/0xfe0
> [ 12.555859] softirqs last enabled at (188): [<c000000000f73504>] .addrconf_verify_rtnl+0x2c4/0xb70
> [ 12.555872] softirqs last disabled at (182): [<c000000000f732b0>] .addrconf_verify_rtnl+0x70/0xb70
> [ 12.555884] CPU: 1 PID: 70 Comm: kworker/1:2 Tainted: G S 6.6.0-rc1 #1
> [ 12.555893] Hardware name: PowerMac7,2 PPC970 0x390202 PowerMac
> [ 12.555898] Workqueue: firewire_ohci .bus_reset_work [firewire_ohci]
> [ 12.555939] Call Trace:
> [ 12.555944] [c000000009677830] [c0000000010d83c0] .dump_stack_lvl+0x8c/0xd0 (unreliable)
> [ 12.555963] [c0000000096778b0] [c000000000140270] .__might_resched+0x320/0x340
> [ 12.555978] [c000000009677940] [c000000000497600] .__kmem_cache_alloc_node+0x390/0x460
> [ 12.555993] [c000000009677a10] [c0000000003fe620] .__kmalloc+0x70/0x310
> [ 12.556007] [c000000009677ac0] [c0003d00004e2268] .fw_core_handle_bus_reset+0x2c8/0xba0 [firewire_core]
> [ 12.556060] [c000000009677c20] [c0003d0000491190] .bus_reset_work+0x330/0x9b0 [firewire_ohci]
> [ 12.556079] [c000000009677d10] [c00000000011d0d0] .process_one_work+0x280/0x6f0
> [ 12.556094] [c000000009677e10] [c00000000011d8a0] .worker_thread+0x360/0x500
> [ 12.556107] [c000000009677ef0] [c00000000012e3b4] .kthread+0x154/0x160
> [ 12.556120] [c000000009677f90] [c00000000000bfa8] .start_kernel_thread+0x10/0x14
>
> John Ogness

Thanks

Takashi Sakamoto