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

Pavel Tikhomirov ptikhomirov at virtuozzo.com
Mon Jul 18 19:28:13 MSK 2022


From: Cyrill Gorcunov <gorcunov at gmail.com>

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 vtty 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_release()
  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 but for our needs this won't work.

Thus before the first 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 appears:
  =============================================
  [ INFO: possible recursive locking detected ]
  3.10.0-1160.42.2.ovz.184.7 #37 Tainted: G         C     ------------
  ---------------------------------------------
  ffff9521b6bc4010 vzctl/8093 is trying to acquire lock:
   (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff8658fec6>] tty_lock+0x86/0xf0

  but task is already holding lock:
   (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff8658fec6>] tty_lock+0x86/0xf0

Note: that lack of proper annotation simply makes lockdep grumpy but there
is no real error.

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

Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>

+++
vtty: lockdep -- adjust slave peer names

The lockdep's helper lockdep_set_subclass() uses first
argument as a key's name and since vtty peers are always
coming in pairs the engine relies that slave's peer locking
primitives are passed with different name. For this sake
lets rename @tty to @o_tty just like it is done in pty
code.

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

Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>

https://jira.sw.ru/browse/PSBM-140913
(cherry picked from vz9 commit 9158b5b5b5d4ff369607ae190c879a7984eb0b1b)
Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

Feature: tty: virtual Container console
---
note: Seems we just forgot to add this patch to vz7.
---
 drivers/tty/pty.c    | 11 ++++++++++-
 drivers/tty/tty_io.c |  3 +++
 include/linux/ve.h   |  2 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 17c5f5173476..785d52200036 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -1257,6 +1257,16 @@ static int __init vtty_init(void)
 	return 0;
 }
 
+void vtty_alloc_tty_struct(const struct tty_driver *driver,
+			   struct tty_struct *o_tty)
+{
+	if (driver != vttys_driver)
+		return;
+
+	tty_set_lock_subclass(o_tty);
+	lockdep_set_subclass(&o_tty->termios_rwsem, TTY_LOCK_SLAVE);
+}
+
 int vtty_open_master(envid_t veid, int idx)
 {
 	struct tty_struct *tty;
@@ -1307,7 +1317,6 @@ int vtty_open_master(envid_t veid, int idx)
 		}
 		tty->count--;
 		tty_unlock(tty);
-		tty_set_lock_subclass(tty);
 		tty = tty->link;
 	}
 
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index a31366b2a898..77c7c5100b3c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3141,6 +3141,9 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
 	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;
 }
 
diff --git a/include/linux/ve.h b/include/linux/ve.h
index 1fc1f8051671..def08dd99a16 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -281,6 +281,8 @@ extern int vtty_open_master(envid_t veid, int idx);
 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 *o_tty);
 #endif /* CONFIG_TTY */
 
 extern struct cgroup *cgroup_get_ve_root(struct cgroup *cgrp);
-- 
2.36.1



More information about the Devel mailing list