Messages in this thread | | | Date | Tue, 22 Aug 2023 07:47:38 +0200 | Subject | Re: [PATCH] xen/xenbus: Avoid a lockdep warning when adding a watch | From | Juergen Gross <> |
| |
On 07.06.23 14:36, Petr Pavlu wrote: > The following lockdep warning appears during boot on a Xen dom0 system: > > [ 96.388794] ====================================================== > [ 96.388797] WARNING: possible circular locking dependency detected > [ 96.388799] 6.4.0-rc5-default+ #8 Tainted: G EL > [ 96.388803] ------------------------------------------------------ > [ 96.388804] xenconsoled/1330 is trying to acquire lock: > [ 96.388808] ffffffff82acdd10 (xs_watch_rwsem){++++}-{3:3}, at: register_xenbus_watch+0x45/0x140 > [ 96.388847] > but task is already holding lock: > [ 96.388849] ffff888100c92068 (&u->msgbuffer_mutex){+.+.}-{3:3}, at: xenbus_file_write+0x2c/0x600 > [ 96.388862] > which lock already depends on the new lock. > > [ 96.388864] > the existing dependency chain (in reverse order) is: > [ 96.388866] > -> #2 (&u->msgbuffer_mutex){+.+.}-{3:3}: > [ 96.388874] __mutex_lock+0x85/0xb30 > [ 96.388885] xenbus_dev_queue_reply+0x48/0x2b0 > [ 96.388890] xenbus_thread+0x1d7/0x950 > [ 96.388897] kthread+0xe7/0x120 > [ 96.388905] ret_from_fork+0x2c/0x50 > [ 96.388914] > -> #1 (xs_response_mutex){+.+.}-{3:3}: > [ 96.388923] __mutex_lock+0x85/0xb30 > [ 96.388930] xenbus_backend_ioctl+0x56/0x1c0 > [ 96.388935] __x64_sys_ioctl+0x90/0xd0 > [ 96.388942] do_syscall_64+0x5c/0x90 > [ 96.388950] entry_SYSCALL_64_after_hwframe+0x72/0xdc > [ 96.388957] > -> #0 (xs_watch_rwsem){++++}-{3:3}: > [ 96.388965] __lock_acquire+0x1538/0x2260 > [ 96.388972] lock_acquire+0xc6/0x2b0 > [ 96.388976] down_read+0x2d/0x160 > [ 96.388983] register_xenbus_watch+0x45/0x140 > [ 96.388990] xenbus_file_write+0x53d/0x600 > [ 96.388994] vfs_write+0xe4/0x490 > [ 96.389003] ksys_write+0xb8/0xf0 > [ 96.389011] do_syscall_64+0x5c/0x90 > [ 96.389017] entry_SYSCALL_64_after_hwframe+0x72/0xdc > [ 96.389023] > other info that might help us debug this: > > [ 96.389025] Chain exists of: > xs_watch_rwsem --> xs_response_mutex --> &u->msgbuffer_mutex > > [ 96.413429] Possible unsafe locking scenario: > > [ 96.413430] CPU0 CPU1 > [ 96.413430] ---- ---- > [ 96.413431] lock(&u->msgbuffer_mutex); > [ 96.413432] lock(xs_response_mutex); > [ 96.413433] lock(&u->msgbuffer_mutex); > [ 96.413434] rlock(xs_watch_rwsem); > [ 96.413436] > *** DEADLOCK *** > > [ 96.413436] 1 lock held by xenconsoled/1330: > [ 96.413438] #0: ffff888100c92068 (&u->msgbuffer_mutex){+.+.}-{3:3}, at: xenbus_file_write+0x2c/0x600 > [ 96.413446] > > An ioctl call IOCTL_XENBUS_BACKEND_SETUP (record #1 in the report) > results in calling xenbus_alloc() -> xs_suspend() which introduces > ordering xs_watch_rwsem --> xs_response_mutex. The xenbus_thread() > operation (record #2) creates xs_response_mutex --> &u->msgbuffer_mutex. > An XS_WATCH write to the xenbus file then results in a complain about > the opposite lock order &u->msgbuffer_mutex --> xs_watch_rwsem. > > The dependency xs_watch_rwsem --> xs_response_mutex is spurious. Avoid > it and the warning by changing the ordering in xs_suspend(), first > acquire xs_response_mutex and then xs_watch_rwsem. Reverse also the > unlocking order in xs_suspend_cancel() for consistency, but keep > xs_resume() as is because it needs to have xs_watch_rwsem unlocked only > after exiting xs suspend and re-adding all watches. > > Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Sorry it took so long,
Juergen
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |