From: Simon Horman <horms@kernel.org>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Christian Marangi <ansuelsmth@gmail.com>,
Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Andrew Lunn <andrew@lunn.ch>,
"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] leds: trigger: netdev: fix RTNL handling to prevent potential deadlock
Date: Fri, 1 Dec 2023 09:54:55 +0000 [thread overview]
Message-ID: <20231201095455.GS32077@kernel.org> (raw)
In-Reply-To: <7cfd7a10-53d4-45a8-908c-39459b1f8b1a@gmail.com>
On Wed, Nov 29, 2023 at 09:03:56AM +0100, Heiner Kallweit wrote:
> When working on LED support for r8169 I got the following lockdep
> warning. Easiest way to prevent this scenario seems to be to take
> the RTNL lock before the trigger_data lock in set_device_name().
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.7.0-rc2-next-20231124+ #2 Not tainted
> ------------------------------------------------------
> bash/383 is trying to acquire lock:
> ffff888103aa1c68 (&trigger_data->lock){+.+.}-{3:3}, at: netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
>
> but task is already holding lock:
> ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (rtnl_mutex){+.+.}-{3:3}:
> __mutex_lock+0x9b/0xb50
> mutex_lock_nested+0x16/0x20
> rtnl_lock+0x12/0x20
> set_device_name+0xa9/0x120 [ledtrig_netdev]
> netdev_trig_activate+0x1a1/0x230 [ledtrig_netdev]
> led_trigger_set+0x172/0x2c0
> led_trigger_write+0xf1/0x140
> sysfs_kf_bin_write+0x5d/0x80
> kernfs_fop_write_iter+0x15d/0x210
> vfs_write+0x1f0/0x510
> ksys_write+0x6c/0xf0
> __x64_sys_write+0x14/0x20
> do_syscall_64+0x3f/0xf0
> entry_SYSCALL_64_after_hwframe+0x6c/0x74
>
> -> #0 (&trigger_data->lock){+.+.}-{3:3}:
> __lock_acquire+0x1459/0x25a0
> lock_acquire+0xc8/0x2d0
> __mutex_lock+0x9b/0xb50
> mutex_lock_nested+0x16/0x20
> netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
> call_netdevice_register_net_notifiers+0x5a/0x100
> register_netdevice_notifier+0x85/0x120
> netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev]
> led_trigger_set+0x172/0x2c0
> led_trigger_write+0xf1/0x140
> sysfs_kf_bin_write+0x5d/0x80
> kernfs_fop_write_iter+0x15d/0x210
> vfs_write+0x1f0/0x510
> ksys_write+0x6c/0xf0
> __x64_sys_write+0x14/0x20
> do_syscall_64+0x3f/0xf0
> entry_SYSCALL_64_after_hwframe+0x6c/0x74
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(rtnl_mutex);
> lock(&trigger_data->lock);
> lock(rtnl_mutex);
> lock(&trigger_data->lock);
>
> *** DEADLOCK ***
>
> 8 locks held by bash/383:
> #0: ffff888103ff33f0 (sb_writers#3){.+.+}-{0:0}, at: ksys_write+0x6c/0xf0
> #1: ffff888103aa1e88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x114/0x210
> #2: ffff8881036f1890 (kn->active#82){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x11d/0x210
> #3: ffff888108e2c358 (&led_cdev->led_access){+.+.}-{3:3}, at: led_trigger_write+0x30/0x140
> #4: ffffffff8cdd9e10 (triggers_list_lock){++++}-{3:3}, at: led_trigger_write+0x75/0x140
> #5: ffff888108e2c270 (&led_cdev->trigger_lock){++++}-{3:3}, at: led_trigger_write+0xe3/0x140
> #6: ffffffff8cdde3d0 (pernet_ops_rwsem){++++}-{3:3}, at: register_netdevice_notifier+0x1c/0x120
> #7: ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20
>
> stack backtrace:
> CPU: 0 PID: 383 Comm: bash Not tainted 6.7.0-rc2-next-20231124+ #2
> Hardware name: Default string Default string/Default string, BIOS ADLN.M6.SODIMM.ZB.CY.015 08/08/2023
> Call Trace:
> <TASK>
> dump_stack_lvl+0x5c/0xd0
> dump_stack+0x10/0x20
> print_circular_bug+0x2dd/0x410
> check_noncircular+0x131/0x150
> __lock_acquire+0x1459/0x25a0
> lock_acquire+0xc8/0x2d0
> ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
> __mutex_lock+0x9b/0xb50
> ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
> ? __this_cpu_preempt_check+0x13/0x20
> ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
> ? __cancel_work_timer+0x11c/0x1b0
> ? __mutex_lock+0x123/0xb50
> mutex_lock_nested+0x16/0x20
> ? mutex_lock_nested+0x16/0x20
> netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
> call_netdevice_register_net_notifiers+0x5a/0x100
> register_netdevice_notifier+0x85/0x120
> netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev]
> led_trigger_set+0x172/0x2c0
> ? preempt_count_add+0x49/0xc0
> led_trigger_write+0xf1/0x140
> sysfs_kf_bin_write+0x5d/0x80
> kernfs_fop_write_iter+0x15d/0x210
> vfs_write+0x1f0/0x510
> ksys_write+0x6c/0xf0
> __x64_sys_write+0x14/0x20
> do_syscall_64+0x3f/0xf0
> entry_SYSCALL_64_after_hwframe+0x6c/0x74
> RIP: 0033:0x7f269055d034
> Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 35 c3 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
> RSP: 002b:00007ffddb7ef748 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 00007f269055d034
> RDX: 0000000000000007 RSI: 000055bf5f4af3c0 RDI: 0000000000000001
> RBP: 000055bf5f4af3c0 R08: 0000000000000073 R09: 0000000000000001
> R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000007
> R13: 00007f26906325c0 R14: 00007f269062ff20 R15: 0000000000000000
> </TASK>
>
> Fixes: f42c437acc55 ("leds: trigger: netdev: add additional specific link speed mode")
Hi Heiner,
The hash above doesn't seem to match what is upstream.
Perhaps it should be:
Fixes: d5e01266e7f5 ("leds: trigger: netdev: add additional specific link speed mode")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
...
next prev parent reply other threads:[~2023-12-01 9:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 8:03 [PATCH] leds: trigger: netdev: fix RTNL handling to prevent potential deadlock Heiner Kallweit
2023-12-01 9:54 ` Simon Horman [this message]
2023-12-01 10:13 ` Heiner Kallweit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231201095455.GS32077@kernel.org \
--to=horms@kernel.org \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=lee@kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pavel@ucw.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.