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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).