[Devel] [PATCH RHEL7 COMMIT] vtty: fix slave peer lockdep annotation

Konstantin Khorenko khorenko at virtuozzo.com
Thu Jul 28 22:08:55 MSK 2022


The commit is pushed to "branch-rh7-3.10.0-1160.66.1.vz7.188.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.66.1.vz7.188.6
------>
commit 26d84bd065b161f719d8b57f1775d45d04fc59b8
Author: Cyrill Gorcunov <gorcunov at gmail.com>
Date:   Mon Jul 18 19:28:13 2022 +0300

    vtty: fix slave peer lockdep annotation
    
    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
---
 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 17c5f51734762..785d52200036f 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 a31366b2a8981..77c7c5100b3cc 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 1fc1f80516712..def08dd99a162 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);


More information about the Devel mailing list