diff mbox series

firewire: core: obsolete usage of GFP_ATOMIC at building node tree

Message ID 20230604070255.172700-1-o-takashi@sakamocchi.jp
State New
Headers
Series firewire: core: obsolete usage of GFP_ATOMIC at building node tree |

Commit Message

Takashi Sakamoto June 4, 2023, 7:02 a.m. UTC 
  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(-)

Comments

Takashi Sakamoto June 5, 2023, 10:56 p.m. UTC | #1 
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
John Ogness Sept. 13, 2023, 9:48 a.m. UTC | #2 
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
Takashi Sakamoto Sept. 15, 2023, 10:04 a.m. UTC | #3 
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
diff mbox series

Patch

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;