[Devel] [PATCH RH9] vtty: fix slave peer lockdep annotation

Cyrill Gorcunov gorcunov at gmail.com
Thu Dec 30 17:34:44 MSK 2021


vtty driver is a tricky one -- we keep slave peers inside container
available for console output even if master peer is not used (unlike
traditional unix terminals where one have to open master peer first
to be able to communicate via multiple slave peers).

Thus our procedure of the driver initialization is quite the reverse
with compare to PTYs: tty_init_dev() is called with @vttys_driver in
first place. What is more interesting is tty_release() procedure: it
locks ttys via tty_lock() call like

tty_lock(tty)
  // o_tty = tty->link;
  tty_lock_slave(o_tty);

and here we need to annotate slave peer mutex that its locking is
nested. The annotation is done via tty_set_lock_subclass() call
but it must be called with pure tty instance never locked before.
So we have a calling sequence

tty_init_dev
   tty = alloc_tty_struct(driver, idx);
   // the driver is pointing to @vttys_driver instance, ie slave peer
   tty_lock(tty);
   // and here we lock the slave peer with mutex without annotation

For regular PTY case it is fine, because the PTYs are always created
with master driver first bot for our needs this won't work.

Thus before the firts tty_lock() call we need to annotate the mutex
and for this sake we introduce vtty_alloc_tty_struct() helper which
get called right after tty instance is allocated.

Without annotation the warning will appear

[  724.408202] =============================================
[  724.408203] [ INFO: possible recursive locking detected ]
[  724.408204] 3.10.0-1160.42.2.ovz.184.7 #37 Tainted: G         C     ------------
[  724.408204] ---------------------------------------------
[  724.408205] ffff9521b6bc4010 vzctl/8093 is trying to acquire lock:
[  724.408215]  (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff8658fec6>] tty_lock+0x86/0xf0
[  724.408216]
but task is already holding lock:
[  724.408219]  (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff8658fec6>] tty_lock+0x86/0xf0

Note that lack of proper annotation simply makes lockdep grumpy but there
is no real errors so no need to backport it I think.

https://jira.sw.ru/browse/PSBM-136773

Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
---
 drivers/tty/pty.c    |   11 ++++++++++-
 drivers/tty/tty_io.c |    3 +++
 include/linux/ve.h   |    2 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

Index: vzkernel/drivers/tty/pty.c
===================================================================
--- vzkernel.orig/drivers/tty/pty.c
+++ vzkernel/drivers/tty/pty.c
@@ -1021,6 +1021,16 @@ static int __init vtty_init(void)
 	return 0;
 }
 
+void vtty_alloc_tty_struct(const struct tty_driver *driver,
+			   struct tty_struct *tty)
+{
+	if (driver != vttys_driver)
+		return;
+
+	tty_set_lock_subclass(tty);
+	lockdep_set_subclass(&tty->termios_rwsem, TTY_LOCK_SLAVE);
+}
+
 int vtty_open_master(envid_t veid, int idx)
 {
 	struct tty_struct *tty;
@@ -1071,7 +1081,6 @@ int vtty_open_master(envid_t veid, int i
 		}
 		tty->count--;
 		tty_unlock(tty);
-		tty_set_lock_subclass(tty);
 		tty = tty->link;
 	}
 
Index: vzkernel/drivers/tty/tty_io.c
===================================================================
--- vzkernel.orig/drivers/tty/tty_io.c
+++ vzkernel/drivers/tty/tty_io.c
@@ -3222,6 +3222,9 @@ struct tty_struct *alloc_tty_struct(stru
 	tty_line_name(driver, idx, tty->name);
 	tty->dev = tty_get_device(tty);
 
+#ifdef CONFIG_TTY
+	vtty_alloc_tty_struct(driver, tty);
+#endif
 	return tty;
 }
 
Index: vzkernel/include/linux/ve.h
===================================================================
--- vzkernel.orig/include/linux/ve.h
+++ vzkernel/include/linux/ve.h
@@ -207,6 +207,8 @@ extern int vtty_open_master(envid_t veid
 extern void vtty_release(struct tty_struct *tty, struct tty_struct *o_tty,
 			int *tty_closing, int *o_tty_closing);
 extern bool vtty_is_master(struct tty_struct *tty);
+extern void vtty_alloc_tty_struct(const struct tty_driver *driver,
+				  struct tty_struct *tty);
 #endif /* CONFIG_TTY */
 
 extern struct cgroup *cgroup_ve_root1(struct cgroup *cgrp);


More information about the Devel mailing list