[PATCH 0/11][v3]: Enable multiple mounts of devpts

[PATCH 0/11][v3]: Enable multiple mounts of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

This patchset includes the patches to cleanup get_node() I had
sent earlier,  with the Sign-off added. No other changes were
made to those patches since I last sent.

---
Enable multiple mounts of devpts filesystem so each container can allocate
ptys independently.

User interface:

	Since supporting multiple mounts of devpts can break user-space, this
	feature is enabled only under a new mount option (-o newinstance).

	If this option is never specified, this patchset should not change
	any behavior.  i.e existing single-namespace semantics are preserved
	across any remounts of /dev/pts (as long as none of them use the new
	option).  So old startup scripts should continue to work.

	If the '-o newinstance' option is specified, then a new 'private' mount
	of devpts is created. Ptys in this private instance are independent
	of ptys created in other devpts instances. For this to be fully
	functional, /dev/ptmx must be a bind-mount of '/dev/pts/ptmx'

	i.e
		mount -t devpts -o newinstance lxcpts /dev/pts
		mount -o bind /dev/pts/ptmx /dev/ptmx

	See [PATCH 11/11] for more details on usage.

Implementation notes:

	1. To enable multiple mounts of /dev/pts, (most) devpts interfaces
	   need to know which instance of devpts is being accessed. This
	   patchset uses the 'struct inode' or 'struct tty_struct' of the
	   device being accessed to identify the appropriate devpts instance.
	   Hence the need for the /dev/pts/ptmx bind-mount.

	2. See comments in get_sb_ref() in fs/super.c (could not find
	   existing interfaces that accomplish it) Or is there a better
	   way ?

	3. Mount options must be parsed twice during mount (once to determine
	   the mode of mount (single/multi-instance) and once to actually
	   save the options. There does not seem to be an easy way to 
	   parse once and reuse (See 'safe_process_mount_opts()' in
	   PATCH 11/11)

	[PATCH 1/11]: Move tty lookup/reopen to tty_open
	[PATCH 2/11]: Add an instance parameter to devpts interfaces
	[PATCH 3/11]: Simplify devpts_get_tty()
	[PATCH 4/11]: Simplify devpts_pty_new()
	[PATCH 5/11]: Simplify devpts_pty_kill
	[PATCH 6/11]: Remove devpts_root global
	[PATCH 7/11]: Per-mount allocated_ptys
	[PATCH 8/11]: Per-mount 'config' object
	[PATCH 9/11]: Extract option parsing to new function
	[PATCH 10/11]: Ability to internally create ptmx 
	[PATCH 11/11]: Enable multiple instances of devpts

Changelog [v3]:

	- Port to 2008-08-28 ttydev tree
	- Rename new mount options to 'ptmxmode' and 'newinstance'.
	- [Alan Cox] Use tty driver data to identify devpts (this is used to
	  cleanup get_node() in devpts_pty_kill()).
	- [H. Peter Anvin] get_node() cleanup in devpts (which was enabled by
	  the inode/tty parameter to devpts interfaces)
	- Bugfix in multi-mount mode (see Patch 11/11).
	- Executed pty tests in LTP (in both single-instance and multi-instance
	  mode)
	- Should be bisect-safe :-)

Changelog [v2]:

	- New mount option '-o newmnt' added (patch 8/8)
	- Support both single-mount and multi-mount semantics (patch 8/8)
	- Automatically create ptmx node when devpts is mounted (patch 7/8)
	- Extract option parsing code to new function (patch 6/8)
	- Make 'config' params per-mount variables (patch 5/8)
	- Slightly re-ordered existing patches in set (patches 1/8, 2/8)

TODO:
	- Do we need a '-o ptmxuid' and '-o ptmxgid' options as well ?
	- Add a config option to enable multiple-mounts of devpts.
	- (Sometime in future) Remove even initial kernel mount of devpts 
	- Any other good test suites to test this (besides LTP, sshd).

[PATCH 1/11][v3]: Move tty lookup/reopen to tty_open

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

Subject: [PATCH 1/11]: Move tty lookup/reopen to tty_open

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Move tty_driver_lookup_tty() and tty_reopen() from tty_init_dev()
into tty_open() (one of the two callers of tty_init_dev()).  These
calls are not really required in ptmx_open(), the other caller,
since ptmx_open() would be setting up a new tty.

Changelog[v2]:
	- remove the lookup and reopen calls from ptmx_open
	- merge with recent changes to ttydev tree

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 drivers/char/tty_io.c |   31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Index: linux-2.6.27-rc3-tty/drivers/char/tty_io.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/drivers/char/tty_io.c	2008-08-28 15:26:34.000000000 -0700
+++ linux-2.6.27-rc3-tty/drivers/char/tty_io.c	2008-08-28 16:16:29.000000000 -0700
@@ -1366,19 +1366,6 @@ struct tty_struct *tty_init_dev(struct t
 	struct tty_struct *tty;
 	int retval;
 
-	/* check whether we're reopening an existing tty */
-	tty = tty_driver_lookup_tty(driver, idx);
-
-	if (IS_ERR(tty))
-		return tty;
-
-	if (tty) {
-		retval = tty_reopen(tty);
-		if (retval)
-			return ERR_PTR(retval);
-		return tty;
-	}
-
 	/* Check if pty master is being opened multiple times */
 	if (driver->subtype == PTY_TYPE_MASTER &&
 		(driver->flags & TTY_DRIVER_DEVPTS_MEM) && !first_ok)
@@ -1780,7 +1767,7 @@ void tty_release_dev(struct file *filp)
 
 static int __tty_open(struct inode *inode, struct file *filp)
 {
-	struct tty_struct *tty;
+	struct tty_struct *tty = NULL;
 	int noctty, retval;
 	struct tty_driver *driver;
 	int index;
@@ -1837,7 +1824,21 @@ retry_open:
 		return -ENODEV;
 	}
 got_driver:
-	tty = tty_init_dev(driver, index, 0);
+	if (!tty) {
+		/* check whether we're reopening an existing tty */
+		tty = tty_driver_lookup_tty(driver, index);
+
+		if (IS_ERR(tty))
+			return PTR_ERR(tty);
+	}
+
+	if (tty) {
+		retval = tty_reopen(tty);
+		if (retval)
+			tty = ERR_PTR(retval);
+	} else
+		tty = tty_init_dev(driver, index, 0);
+
 	mutex_unlock(&tty_mutex);
 	tty_driver_kref_put(driver);
 	if (IS_ERR(tty))

[PATCH 2/11][v3]: Add an instance parameter devpts interfaces

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 2/11]: Add an instance parameter to devpts interfaces

Pass-in 'inode' or 'tty' parameter to devpts interfaces.  With multiple
devpts instances, these parameters will be used in subsequent patches
to identify the instance of devpts mounted. The parameters also help
simplify devpts implementation.

Changelog[v3]:  
	- minor changes due to merge with ttydev updates
	- rename parameters to emphasize they are ptmx or pts inodes
	- pass-in 'tty_struct*' to devpts_pty_kill() (this will help
	  cleanup the get_node() call in a subsequent patch)

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 drivers/char/pty.c         |   18 ++++++++++--------
 drivers/char/tty_io.c      |   14 ++++++++------
 fs/devpts/inode.c          |   11 ++++++-----
 include/linux/devpts_fs.h  |   31 +++++++++++++++++++++----------
 include/linux/tty_driver.h |    3 ++-
 5 files changed, 47 insertions(+), 30 deletions(-)

Index: linux-2.6.27-rc3-tty/drivers/char/pty.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/drivers/char/pty.c	2008-08-28 16:16:29.000000000 -0700
+++ linux-2.6.27-rc3-tty/drivers/char/pty.c	2008-08-28 16:17:04.000000000 -0700
@@ -60,7 +60,7 @@ static void pty_close(struct tty_struct 
 		set_bit(TTY_OTHER_CLOSED, &tty->flags);
 #ifdef CONFIG_UNIX98_PTYS
 		if (tty->driver == ptm_driver)
-			devpts_pty_kill(tty->index);
+			devpts_pty_kill(tty->link);
 #endif
 		tty_vhangup(tty->link);
 	}
@@ -451,9 +451,10 @@ static int pty_unix98_ioctl(struct tty_s
  *	This provides our locking.
  */
 
-static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver, int idx)
+static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
+		struct inode *ptm_inode, int idx)
 {
-	struct tty_struct *tty = devpts_get_tty(idx);
+	struct tty_struct *tty = devpts_get_tty(ptm_inode, idx);
 	if (tty)
 		tty = tty->link;
 	return tty;
@@ -468,9 +469,10 @@ static struct tty_struct *ptm_unix98_loo
  *	This provides our locking.
  */
 
-static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver, int idx)
+static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
+		struct inode *pts_inode, int idx)
 {
-	struct tty_struct *tty = devpts_get_tty(idx);
+	struct tty_struct *tty = devpts_get_tty(pts_inode, idx);
 	/* Master must be open before slave */
 	if (!tty)
 		return ERR_PTR(-EIO);
@@ -598,7 +600,7 @@ static int __ptmx_open(struct inode *ino
 	nonseekable_open(inode, filp);
 
 	/* find a device that is not in use. */
-	index = devpts_new_index();
+	index = devpts_new_index(inode);
 	if (index < 0)
 		return index;
 
@@ -615,7 +617,7 @@ static int __ptmx_open(struct inode *ino
 	filp->private_data = tty;
 	file_move(filp, &tty->tty_files);
 
-	retval = devpts_pty_new(tty->link);
+	retval = devpts_pty_new(inode, tty->link);
 	if (retval)
 		goto out1;
 
@@ -626,7 +628,7 @@ out1:
 	tty_release_dev(filp);
 	return retval;
 out:
-	devpts_kill_index(index);
+	devpts_kill_index(inode, index);
 	return retval;
 }
 
Index: linux-2.6.27-rc3-tty/drivers/char/tty_io.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/drivers/char/tty_io.c	2008-08-28 16:16:29.000000000 -0700
+++ linux-2.6.27-rc3-tty/drivers/char/tty_io.c	2008-08-28 16:17:59.000000000 -0700
@@ -1203,12 +1203,13 @@ static void tty_line_name(struct tty_dri
  *	be held until the 'fast-open' is also done. Will change once we
  *	have refcounting in the driver and per driver locking
  */
-struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver, int idx)
+struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver,
+		struct inode *inode, int idx)
 {
 	struct tty_struct *tty;
 
 	if (driver->ops->lookup)
-		return driver->ops->lookup(driver, idx);
+		return driver->ops->lookup(driver, inode, idx);
 
 	tty = driver->ttys[idx];
 	return tty;
@@ -1529,10 +1530,11 @@ void tty_release_dev(struct file *filp)
 	int	devpts;
 	int	idx;
 	char	buf[64];
+	struct 	inode *inode;
 
+	inode = filp->f_path.dentry->d_inode;
 	tty = (struct tty_struct *)filp->private_data;
-	if (tty_paranoia_check(tty, filp->f_path.dentry->d_inode,
-							"tty_release_dev"))
+	if (tty_paranoia_check(tty, inode, "tty_release_dev"))
 		return;
 
 	check_tty_count(tty, "tty_release_dev");
@@ -1741,7 +1743,7 @@ void tty_release_dev(struct file *filp)
 
 	/* Make this pty number available for reallocation */
 	if (devpts)
-		devpts_kill_index(idx);
+		devpts_kill_index(inode, idx);
 }
 
 /**
@@ -1826,7 +1828,7 @@ retry_open:
 got_driver:
 	if (!tty) {
 		/* check whether we're reopening an existing tty */
-		tty = tty_driver_lookup_tty(driver, index);
+		tty = tty_driver_lookup_tty(driver, inode, index);
 
 		if (IS_ERR(tty))
 			return PTR_ERR(tty);
Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-08-28 16:16:29.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-08-28 17:59:15.000000000 -0700
@@ -177,7 +177,7 @@ static struct dentry *get_node(int num)
 	return lookup_one_len(s, root, sprintf(s, "%d", num));
 }
 
-int devpts_new_index(void)
+int devpts_new_index(struct inode *ptmx_inode)
 {
 	int index;
 	int ida_ret;
@@ -205,14 +205,14 @@ retry:
 	return index;
 }
 
-void devpts_kill_index(int idx)
+void devpts_kill_index(struct inode *ptmx_inode, int idx)
 {
 	mutex_lock(&allocated_ptys_lock);
 	ida_remove(&allocated_ptys, idx);
 	mutex_unlock(&allocated_ptys_lock);
 }
 
-int devpts_pty_new(struct tty_struct *tty)
+int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty)
 {
 	int number = tty->index; /* tty layer puts index from devpts_new_index() in here */
 	struct tty_driver *driver = tty->driver;
@@ -245,7 +245,7 @@ int devpts_pty_new(struct tty_struct *tt
 	return 0;
 }
 
-struct tty_struct *devpts_get_tty(int number)
+struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
 {
 	struct dentry *dentry = get_node(number);
 	struct tty_struct *tty;
@@ -262,8 +262,9 @@ struct tty_struct *devpts_get_tty(int nu
 	return tty;
 }
 
-void devpts_pty_kill(int number)
+void devpts_pty_kill(struct tty_struct *tty)
 {
+	int number = tty->index;
 	struct dentry *dentry = get_node(number);
 
 	if (!IS_ERR(dentry)) {
Index: linux-2.6.27-rc3-tty/include/linux/devpts_fs.h
===================================================================
--- linux-2.6.27-rc3-tty.orig/include/linux/devpts_fs.h	2008-08-28 16:16:29.000000000 -0700
+++ linux-2.6.27-rc3-tty/include/linux/devpts_fs.h	2008-08-28 16:17:04.000000000 -0700
@@ -17,20 +17,31 @@
 
 #ifdef CONFIG_UNIX98_PTYS
 
-int devpts_new_index(void);
-void devpts_kill_index(int idx);
-int devpts_pty_new(struct tty_struct *tty);      /* mknod in devpts */
-struct tty_struct *devpts_get_tty(int number);	 /* get tty structure */
-void devpts_pty_kill(int number);		 /* unlink */
+int devpts_new_index(struct inode *ptmx_inode);
+void devpts_kill_index(struct inode *ptmx_inode, int idx);
+/* mknod in devpts */
+int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
+/* get tty structure */
+struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number);
+/* unlink */
+void devpts_pty_kill(struct tty_struct *tty);
 
 #else
 
 /* Dummy stubs in the no-pty case */
-static inline int devpts_new_index(void) { return -EINVAL; }
-static inline void devpts_kill_index(int idx) { }
-static inline int devpts_pty_new(struct tty_struct *tty) { return -EINVAL; }
-static inline struct tty_struct *devpts_get_tty(int number) { return NULL; }
-static inline void devpts_pty_kill(int number) { }
+static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
+static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
+static inline int devpts_pty_new(struct inode *ptmx_inode,
+				struct tty_struct *tty)
+{
+	return -EINVAL;
+}
+static inline struct tty_struct *devpts_get_tty(struct inode *pts_inode,
+		int number)
+{
+	return NULL;
+}
+static inline void devpts_pty_kill(struct tty_struct *tty) { }
 
 #endif
 
Index: linux-2.6.27-rc3-tty/include/linux/tty_driver.h
===================================================================
--- linux-2.6.27-rc3-tty.orig/include/linux/tty_driver.h	2008-08-28 16:16:29.000000000 -0700
+++ linux-2.6.27-rc3-tty/include/linux/tty_driver.h	2008-08-28 16:17:04.000000000 -0700
@@ -225,7 +225,8 @@ struct tty_struct;
 struct tty_driver;
 
 struct tty_operations {
-	struct tty_struct * (*lookup)(struct tty_driver *driver, int idx);
+	struct tty_struct * (*lookup)(struct tty_driver *driver,
+			struct inode *inode, int idx);
 	int  (*install)(struct tty_driver *driver, struct tty_struct *tty);
 	void (*remove)(struct tty_driver *driver, struct tty_struct *tty);
 	int  (*open)(struct tty_struct * tty, struct file * filp);

[PATCH 3/11][v3]: Simplify devpts_get_tty()

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 3/11]: Simplify devpts_get_tty()

As pointed out by H. Peter Anvin, since the inode for the pty is known,
we don't need to look it up.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-08-28 17:59:15.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-08-28 18:01:11.000000000 -0700
@@ -27,6 +27,7 @@
 #define DEVPTS_SUPER_MAGIC 0x1cd1
 
 #define DEVPTS_DEFAULT_MODE 0600
+#define PTMX_MINOR	2
 
 extern int pty_limit;			/* Config limit on Unix98 ptys */
 static DEFINE_IDA(allocated_ptys);
@@ -247,19 +248,11 @@ int devpts_pty_new(struct inode *ptmx_in
 
 struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
 {
-	struct dentry *dentry = get_node(number);
-	struct tty_struct *tty;
-
-	tty = NULL;
-	if (!IS_ERR(dentry)) {
-		if (dentry->d_inode)
-			tty = dentry->d_inode->i_private;
-		dput(dentry);
-	}
-
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
 
-	return tty;
+	if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
+		return (struct tty_struct *)pts_inode->i_private;
+	return NULL;
 }
 
 void devpts_pty_kill(struct tty_struct *tty)

[PATCH 4/11][v3]: Simplify devpts_pty_new()

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 4/11]: Simplify devpts_pty_new()

devpts_pty_new() is called when setting up a new pty and would not
have an existing dentry or inode for the pty. So don't bother looking
for an existing dentry - just create a new one.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-08-28 18:01:11.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-08-28 18:46:55.000000000 -0700
@@ -220,6 +220,7 @@ int devpts_pty_new(struct inode *ptmx_in
 	dev_t device = MKDEV(driver->major, driver->minor_start+number);
 	struct dentry *dentry;
 	struct inode *inode = new_inode(devpts_mnt->mnt_sb);
+	char s[12];
 
 	/* We're supposed to be given the slave end of a pty */
 	BUG_ON(driver->type != TTY_DRIVER_TYPE_PTY);
@@ -235,9 +236,13 @@ int devpts_pty_new(struct inode *ptmx_in
 	init_special_inode(inode, S_IFCHR|config.mode, device);
 	inode->i_private = tty;
 
-	dentry = get_node(number);
-	if (!IS_ERR(dentry) && !dentry->d_inode) {
-		d_instantiate(dentry, inode);
+	sprintf(s, "%d", number);
+
+	mutex_lock(&devpts_root->d_inode->i_mutex);
+
+	dentry = d_alloc_name(devpts_root, s);
+	if (!IS_ERR(dentry)) {
+		d_add(dentry, inode);
 		fsnotify_create(devpts_root->d_inode, dentry);
 	}

[PATCH 5/11][v3]: Simplify devpts_pty_kill

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 5/11]: Simplify devpts_pty_kill

When creating a new pty, save the pty's inode in the tty->driver_data.
Use this inode in pty_kill() to identify the devpts instance. Since
we now have the inode for the pty, we can skip get_node() lookup and
remove the unused get_node().

TODO: 
	- check if the mutex_lock is needed in pty_kill().

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-08-28 18:46:55.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-08-28 23:27:20.000000000 -0700
@@ -170,14 +170,6 @@ static struct file_system_type devpts_fs
  * to the System V naming convention
  */
 
-static struct dentry *get_node(int num)
-{
-	char s[12];
-	struct dentry *root = devpts_root;
-	mutex_lock(&root->d_inode->i_mutex);
-	return lookup_one_len(s, root, sprintf(s, "%d", num));
-}
-
 int devpts_new_index(struct inode *ptmx_inode)
 {
 	int index;
@@ -235,6 +227,7 @@ int devpts_pty_new(struct inode *ptmx_in
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	init_special_inode(inode, S_IFCHR|config.mode, device);
 	inode->i_private = tty;
+	tty->driver_data = inode;
 
 	sprintf(s, "%d", number);
 
@@ -262,18 +255,20 @@ struct tty_struct *devpts_get_tty(struct
 
 void devpts_pty_kill(struct tty_struct *tty)
 {
-	int number = tty->index;
-	struct dentry *dentry = get_node(number);
+	struct inode *inode = tty->driver_data;
+	struct dentry *dentry;
 
-	if (!IS_ERR(dentry)) {
-		struct inode *inode = dentry->d_inode;
-		if (inode) {
-			inode->i_nlink--;
-			d_delete(dentry);
-			dput(dentry);
-		}
+	BUG_ON(inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
+
+	mutex_lock(&devpts_root->d_inode->i_mutex);
+
+	dentry = d_find_alias(inode);
+	if (dentry && !IS_ERR(dentry)) {
+		inode->i_nlink--;
+		d_delete(dentry);
 		dput(dentry);
 	}
+
 	mutex_unlock(&devpts_root->d_inode->i_mutex);
 }

[PATCH 6/11][v3]: Remove devpts_root global

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

Serge, Matt, 

	Pls sign-off if you agree/want to.

---
From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 6/11]: Remove devpts_root global

Remove the 'devpts_root' global variable and find the root dentry using
the super_block. The super-block can be found from the device inode, using
the new wrapper, pts_sb_from_inode().

Changelog: This patch is based on an earlier patchset from Serge Hallyn
	   and Matt Helsley.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-08-28 23:27:20.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:31:08.000000000 -0700
@@ -34,7 +34,6 @@ static DEFINE_IDA(allocated_ptys);
 static DEFINE_MUTEX(allocated_ptys_lock);
 
 static struct vfsmount *devpts_mnt;
-static struct dentry *devpts_root;
 
 static struct {
 	int setuid;
@@ -56,6 +55,14 @@ static match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
+static inline struct super_block *pts_sb_from_inode(struct inode *inode)
+{
+	if (inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
+		return inode->i_sb;
+
+	return devpts_mnt->mnt_sb;
+}
+
 static int devpts_remount(struct super_block *sb, int *flags, char *data)
 {
 	char *p;
@@ -142,7 +149,7 @@ devpts_fill_super(struct super_block *s,
 	inode->i_fop = &simple_dir_operations;
 	inode->i_nlink = 2;
 
-	devpts_root = s->s_root = d_alloc_root(inode);
+	s->s_root = d_alloc_root(inode);
 	if (s->s_root)
 		return 0;
 	
@@ -211,7 +218,9 @@ int devpts_pty_new(struct inode *ptmx_in
 	struct tty_driver *driver = tty->driver;
 	dev_t device = MKDEV(driver->major, driver->minor_start+number);
 	struct dentry *dentry;
-	struct inode *inode = new_inode(devpts_mnt->mnt_sb);
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+	struct inode *inode = new_inode(sb);
+	struct dentry *root = sb->s_root;
 	char s[12];
 
 	/* We're supposed to be given the slave end of a pty */
@@ -231,15 +240,15 @@ int devpts_pty_new(struct inode *ptmx_in
 
 	sprintf(s, "%d", number);
 
-	mutex_lock(&devpts_root->d_inode->i_mutex);
+	mutex_lock(&root->d_inode->i_mutex);
 
-	dentry = d_alloc_name(devpts_root, s);
+	dentry = d_alloc_name(root, s);
 	if (!IS_ERR(dentry)) {
 		d_add(dentry, inode);
-		fsnotify_create(devpts_root->d_inode, dentry);
+		fsnotify_create(root->d_inode, dentry);
 	}
 
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 
 	return 0;
 }
@@ -256,11 +265,13 @@ struct tty_struct *devpts_get_tty(struct
 void devpts_pty_kill(struct tty_struct *tty)
 {
 	struct inode *inode = tty->driver_data;
+	struct super_block *sb = pts_sb_from_inode(inode);
+	struct dentry *root = sb->s_root;
 	struct dentry *dentry;
 
 	BUG_ON(inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
 
-	mutex_lock(&devpts_root->d_inode->i_mutex);
+	mutex_lock(&root->d_inode->i_mutex);
 
 	dentry = d_find_alias(inode);
 	if (dentry && !IS_ERR(dentry)) {
@@ -269,7 +280,7 @@ void devpts_pty_kill(struct tty_struct *
 		dput(dentry);
 	}
 
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 }
 
 static int __init init_devpts_fs(void)

[PATCH 7/11][v3]: Per-mount allocated_ptys

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 7/11]: Per-mount allocated_ptys

To enable multiple mounts of devpts, 'allocated_ptys' must be a per-mount
variable rather than a global variable.  Move 'allocated_ptys' into the
super_block's s_fs_info.

Changelog[v2]:
	Define and use DEVPTS_SB() wrapper.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 7 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-09-03 21:31:08.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:32:10.000000000 -0700
@@ -30,7 +30,6 @@
 #define PTMX_MINOR	2
 
 extern int pty_limit;			/* Config limit on Unix98 ptys */
-static DEFINE_IDA(allocated_ptys);
 static DEFINE_MUTEX(allocated_ptys_lock);
 
 static struct vfsmount *devpts_mnt;
@@ -55,6 +54,15 @@ static match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
+struct pts_fs_info {
+	struct ida allocated_ptys;
+};
+
+static inline struct pts_fs_info * DEVPTS_SB(struct super_block *sb)
+{
+	return sb->s_fs_info;
+}
+
 static inline struct super_block *pts_sb_from_inode(struct inode *inode)
 {
 	if (inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
@@ -126,6 +134,18 @@ static const struct super_operations dev
 	.show_options	= devpts_show_options,
 };
 
+static void *new_pts_fs_info(void)
+{
+	struct pts_fs_info *fsi;
+
+	fsi = kmalloc(sizeof(struct pts_fs_info), GFP_KERNEL);
+	if (fsi)
+		ida_init(&fsi->allocated_ptys);
+
+	return fsi;
+}
+
+
 static int
 devpts_fill_super(struct super_block *s, void *data, int silent)
 {
@@ -137,9 +157,13 @@ devpts_fill_super(struct super_block *s,
 	s->s_op = &devpts_sops;
 	s->s_time_gran = 1;
 
+	s->s_fs_info = new_pts_fs_info();
+	if (!s->s_fs_info)
+		goto fail;
+
 	inode = new_inode(s);
 	if (!inode)
-		goto fail;
+		goto free_fsi;
 	inode->i_ino = 1;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	inode->i_blocks = 0;
@@ -155,6 +179,9 @@ devpts_fill_super(struct super_block *s,
 	
 	printk("devpts: get root dentry failed\n");
 	iput(inode);
+
+free_fsi:
+	kfree(s->s_fs_info);
 fail:
 	return -ENOMEM;
 }
@@ -165,11 +192,20 @@ static int devpts_get_sb(struct file_sys
 	return get_sb_single(fs_type, flags, data, devpts_fill_super, mnt);
 }
 
+
+static void devpts_kill_sb(struct super_block *sb)
+{
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);
+
+	kfree(fsi);
+	kill_anon_super(sb);
+}
+
 static struct file_system_type devpts_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "devpts",
 	.get_sb		= devpts_get_sb,
-	.kill_sb	= kill_anon_super,
+	.kill_sb	= devpts_kill_sb,
 };
 
 /*
@@ -179,16 +215,18 @@ static struct file_system_type devpts_fs
 
 int devpts_new_index(struct inode *ptmx_inode)
 {
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);        // need rcu ?
 	int index;
 	int ida_ret;
 
 retry:
-	if (!ida_pre_get(&allocated_ptys, GFP_KERNEL)) {
+	if (!ida_pre_get(&fsi->allocated_ptys, GFP_KERNEL)) {
 		return -ENOMEM;
 	}
 
 	mutex_lock(&allocated_ptys_lock);
-	ida_ret = ida_get_new(&allocated_ptys, &index);
+	ida_ret = ida_get_new(&fsi->allocated_ptys, &index);
 	if (ida_ret < 0) {
 		mutex_unlock(&allocated_ptys_lock);
 		if (ida_ret == -EAGAIN)
@@ -197,7 +235,7 @@ retry:
 	}
 
 	if (index >= pty_limit) {
-		ida_remove(&allocated_ptys, index);
+		ida_remove(&fsi->allocated_ptys, index);
 		mutex_unlock(&allocated_ptys_lock);
 		return -EIO;
 	}
@@ -207,8 +245,11 @@ retry:
 
 void devpts_kill_index(struct inode *ptmx_inode, int idx)
 {
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);        // need rcu ?
+
 	mutex_lock(&allocated_ptys_lock);
-	ida_remove(&allocated_ptys, idx);
+	ida_remove(&fsi->allocated_ptys, idx);
 	mutex_unlock(&allocated_ptys_lock);
 }

[PATCH 8/11][v3]: Per-mount 'config' object

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 8/11]: Per-mount 'config' object

With support for multiple mounts of devpts, the 'config' structure really
represents per-mount options rather than config parameters. Rename 'config'
structure to 'pts_mount_opts' and store it in the super-block.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   57 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-09-03 21:32:10.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:33:36.000000000 -0700
@@ -34,13 +34,13 @@ static DEFINE_MUTEX(allocated_ptys_lock)
 
 static struct vfsmount *devpts_mnt;
 
-static struct {
+struct pts_mount_opts {
 	int setuid;
 	int setgid;
 	uid_t   uid;
 	gid_t   gid;
 	umode_t mode;
-} config = {.mode = DEVPTS_DEFAULT_MODE};
+};
 
 enum {
 	Opt_uid, Opt_gid, Opt_mode,
@@ -56,6 +56,7 @@ static match_table_t tokens = {
 
 struct pts_fs_info {
 	struct ida allocated_ptys;
+	struct pts_mount_opts mount_opts;
 };
 
 static inline struct pts_fs_info * DEVPTS_SB(struct super_block *sb)
@@ -74,12 +75,14 @@ static inline struct super_block *pts_sb
 static int devpts_remount(struct super_block *sb, int *flags, char *data)
 {
 	char *p;
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);
+	struct pts_mount_opts *opts = &fsi->mount_opts;
 
-	config.setuid  = 0;
-	config.setgid  = 0;
-	config.uid     = 0;
-	config.gid     = 0;
-	config.mode    = DEVPTS_DEFAULT_MODE;
+	opts->setuid  = 0;
+	opts->setgid  = 0;
+	opts->uid     = 0;
+	opts->gid     = 0;
+	opts->mode    = DEVPTS_DEFAULT_MODE;
 
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -94,19 +97,19 @@ static int devpts_remount(struct super_b
 		case Opt_uid:
 			if (match_int(&args[0], &option))
 				return -EINVAL;
-			config.uid = option;
-			config.setuid = 1;
+			opts->uid = option;
+			opts->setuid = 1;
 			break;
 		case Opt_gid:
 			if (match_int(&args[0], &option))
 				return -EINVAL;
-			config.gid = option;
-			config.setgid = 1;
+			opts->gid = option;
+			opts->setgid = 1;
 			break;
 		case Opt_mode:
 			if (match_octal(&args[0], &option))
 				return -EINVAL;
-			config.mode = option & S_IALLUGO;
+			opts->mode = option & S_IALLUGO;
 			break;
 		default:
 			printk(KERN_ERR "devpts: called with bogus options\n");
@@ -119,11 +122,14 @@ static int devpts_remount(struct super_b
 
 static int devpts_show_options(struct seq_file *seq, struct vfsmount *vfs)
 {
-	if (config.setuid)
-		seq_printf(seq, ",uid=%u", config.uid);
-	if (config.setgid)
-		seq_printf(seq, ",gid=%u", config.gid);
-	seq_printf(seq, ",mode=%03o", config.mode);
+	struct pts_fs_info *fsi = DEVPTS_SB(vfs->mnt_sb);
+	struct pts_mount_opts *opts = &fsi->mount_opts;
+
+	if (opts->setuid)
+		seq_printf(seq, ",uid=%u", opts->uid);
+	if (opts->setgid)
+		seq_printf(seq, ",gid=%u", opts->gid);
+	seq_printf(seq, ",mode=%03o", opts->mode);
 
 	return 0;
 }
@@ -138,9 +144,12 @@ static void *new_pts_fs_info(void)
 {
 	struct pts_fs_info *fsi;
 
-	fsi = kmalloc(sizeof(struct pts_fs_info), GFP_KERNEL);
-	if (fsi)
-		ida_init(&fsi->allocated_ptys);
+	fsi = kzalloc(sizeof(struct pts_fs_info), GFP_KERNEL);
+	if (!fsi)
+		return NULL;
+
+	ida_init(&fsi->allocated_ptys);
+	fsi->mount_opts.mode = DEVPTS_DEFAULT_MODE;
 
 	return fsi;
 }
@@ -262,6 +271,8 @@ int devpts_pty_new(struct inode *ptmx_in
 	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
 	struct inode *inode = new_inode(sb);
 	struct dentry *root = sb->s_root;
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);
+	struct pts_mount_opts *opts = &fsi->mount_opts;
 	char s[12];
 
 	/* We're supposed to be given the slave end of a pty */
@@ -272,10 +283,10 @@ int devpts_pty_new(struct inode *ptmx_in
 		return -ENOMEM;
 
 	inode->i_ino = number+2;
-	inode->i_uid = config.setuid ? config.uid : current->fsuid;
-	inode->i_gid = config.setgid ? config.gid : current->fsgid;
+	inode->i_uid = opts->setuid ? opts->uid : current->fsuid;
+	inode->i_gid = opts->setgid ? opts->gid : current->fsgid;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	init_special_inode(inode, S_IFCHR|config.mode, device);
+	init_special_inode(inode, S_IFCHR|opts->mode, device);
 	inode->i_private = tty;
 	tty->driver_data = inode;

[PATCH 9/11][v3]: Extract option parsing to new function

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 9/11]: Extract option parsing to new function

Move code to parse mount options into a separate function so it can
(later) be shared between mount and remount operations.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-09-03 21:33:36.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:33:46.000000000 -0700
@@ -72,11 +72,9 @@ static inline struct super_block *pts_sb
 	return devpts_mnt->mnt_sb;
 }
 
-static int devpts_remount(struct super_block *sb, int *flags, char *data)
+static int parse_mount_options(char *data, struct pts_mount_opts *opts)
 {
 	char *p;
-	struct pts_fs_info *fsi = DEVPTS_SB(sb);
-	struct pts_mount_opts *opts = &fsi->mount_opts;
 
 	opts->setuid  = 0;
 	opts->setgid  = 0;
@@ -120,6 +118,14 @@ static int devpts_remount(struct super_b
 	return 0;
 }
 
+static int devpts_remount(struct super_block *sb, int *flags, char *data)
+{
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);
+	struct pts_mount_opts *opts = &fsi->mount_opts;
+
+	return parse_mount_options(data, opts);
+}
+
 static int devpts_show_options(struct seq_file *seq, struct vfsmount *vfs)
 {
 	struct pts_fs_info *fsi = DEVPTS_SB(vfs->mnt_sb);

[PATCH 10/11][v3]: Ability to internally create ptmx

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 10/11]: Ability to internally create ptmx 

/dev/ptmx is closely tied to the devpts filesystem. An open of /dev/ptmx,
allocates the next pty index and the associated device shows up in the
devpts fs as /dev/pts/n.

Wih multiple instancs of devpts filesystem, during an open of /dev/ptmx
we would be unable to determine which instance of the devpts is being
accessed.

So we move the 'ptmx' node into /dev/pts and use the inode of the 'ptmx'
node to identify the superblock and hence the devpts instance.  This patch
adds ability for the kernel to internally create the [ptmx, c, 5:2] device
when mounting devpts filesystem. The permissions for the device node can
be specified by the '-o ptmxmode=0666' option.  The default 'ptmxmode' is
0666.

See next patch ("PATCH [11/11] Enable multiple instances of devpts") for
usage of this interface, user-space impact and backward compatibility issues


Changelog[v3]:
	- Rename ptmx_mode to ptmxmode (for consistency with 'newinstance')

Changelog[v2]:
	- [H. Peter Anvin] Remove mknod() system call support and create the
	  ptmx node internally.

Changelog[v1]:
	- Earlier version of this patch enabled creating /dev/pts/tty as
	  well. As pointed out by Al Viro and H. Peter Anvin, that is not
	  really necessary.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-09-03 21:33:46.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:34:36.000000000 -0700
@@ -27,6 +27,7 @@
 #define DEVPTS_SUPER_MAGIC 0x1cd1
 
 #define DEVPTS_DEFAULT_MODE 0600
+#define DEVPTS_DEFAULT_PTMX_MODE	0666
 #define PTMX_MINOR	2
 
 extern int pty_limit;			/* Config limit on Unix98 ptys */
@@ -40,10 +41,11 @@ struct pts_mount_opts {
 	uid_t   uid;
 	gid_t   gid;
 	umode_t mode;
+	umode_t ptmxmode;
 };
 
 enum {
-	Opt_uid, Opt_gid, Opt_mode,
+	Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode,
 	Opt_err
 };
 
@@ -51,6 +53,7 @@ static match_table_t tokens = {
 	{Opt_uid, "uid=%u"},
 	{Opt_gid, "gid=%u"},
 	{Opt_mode, "mode=%o"},
+	{Opt_ptmxmode, "ptmxmode=%o"},
 	{Opt_err, NULL}
 };
 
@@ -81,6 +84,7 @@ static int parse_mount_options(char *dat
 	opts->uid     = 0;
 	opts->gid     = 0;
 	opts->mode    = DEVPTS_DEFAULT_MODE;
+	opts->ptmxmode = DEVPTS_DEFAULT_PTMX_MODE;
 
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -109,6 +113,11 @@ static int parse_mount_options(char *dat
 				return -EINVAL;
 			opts->mode = option & S_IALLUGO;
 			break;
+		case Opt_ptmxmode:
+			if (match_octal(&args[0], &option))
+				return -EINVAL;
+			opts->ptmxmode = option & S_IALLUGO;
+			break;
 		default:
 			printk(KERN_ERR "devpts: called with bogus options\n");
 			return -EINVAL;
@@ -136,6 +145,7 @@ static int devpts_show_options(struct se
 	if (opts->setgid)
 		seq_printf(seq, ",gid=%u", opts->gid);
 	seq_printf(seq, ",mode=%03o", opts->mode);
+	seq_printf(seq, ",ptmxmode=%03o", opts->ptmxmode);
 
 	return 0;
 }
@@ -156,6 +166,7 @@ static void *new_pts_fs_info(void)
 
 	ida_init(&fsi->allocated_ptys);
 	fsi->mount_opts.mode = DEVPTS_DEFAULT_MODE;
+	fsi->mount_opts.ptmxmode = DEVPTS_DEFAULT_PTMX_MODE;
 
 	return fsi;
 }
@@ -201,6 +212,53 @@ fail:
 	return -ENOMEM;
 }
 
+int mknod_ptmx(struct super_block *sb)
+{
+	struct dentry *root;
+	struct dentry *dentry;
+	struct inode *inode;
+	struct pts_fs_info *fsi = DEVPTS_SB(sb);
+	struct pts_mount_opts *opts = &fsi->mount_opts;
+	int mode;
+
+	root = sb->s_root;
+	dentry = lookup_one_len("ptmx", root, 4);
+	if (IS_ERR(dentry)) {
+		printk(KERN_ERR "Unable to alloc dentry for ptmx node\n");
+		return -ENOMEM;
+	}
+
+	if (dentry->d_inode) {
+		printk(KERN_ERR "'ptmx' (ino %lu) exists in this devpts\n",
+				dentry->d_inode->i_ino);
+		return 0;
+	}
+
+	/*
+	 * Create a new 'ptmx' node in this mount of devpts.
+	 */
+	inode = new_inode(sb);
+	if (!inode) {
+		printk(KERN_ERR "Unable to alloc inode for ptmx node\n");
+		dput(dentry);
+		return -ENOMEM;
+	}
+
+	inode->i_uid = inode->i_gid = 0;
+	inode->i_blocks = 0;
+	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
+
+	mode = S_IFCHR|opts->ptmxmode;
+	init_special_inode(inode, mode, MKDEV(TTYAUX_MAJOR, 2));
+
+	d_add(dentry, inode);	// d_instantiate() should be enough ?
+
+	printk(KERN_DEBUG "Created ptmx node in devpts ino %lu\n",
+			inode->i_ino);
+
+	return 0;
+}
+
 static int devpts_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
@@ -213,7 +271,7 @@ static void devpts_kill_sb(struct super_
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
 
 	kfree(fsi);
-	kill_anon_super(sb);
+	kill_litter_super(sb);
 }
 
 static struct file_system_type devpts_fs_type = {

[PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH 11/11]: Enable multiple instances of devpts

To support containers, allow multiple instances of devpts filesystem.
such that indices of ptys allocated in one instance are independent
of ptys allocated in other instances of devpts.

But to preserve backward compatibility, enable this support for multiple
instances under the new mount option, '-o newinstance'.

IOW, devpts must support both single-mount and multiple-mount semantics.
If the filesystem is mounted without the 'newinstance' option (as in current
start-up scripts) the new mount simply binds to the initial kernel mount
of devpts and thus current behavior is preserved.

If the 'newinstance' option is specified (by new startup scripts) a new
instance of the devpts fs is created and any ptys created in this instance
are independent of the ptys in other mounts of devpts.

Eg: A container startup script could do the following:

	$ ns_exec -cm /bin/bash
	$ umount /dev/pts
	$ mount -t devpts -o newinstance lxcpts /dev/pts
	$ mount -o bind /dev/pts/ptmx /dev/ptmx
	$ sshd -p 6710

where 'ns_exec -cm /bin/bash' is calls clone() with CLONE_NEWNS flag
and execs /bin/bash in the child process.  A pty created by the sshd
is not visible in the original mount of /dev/pts.

USER-SPACE-IMPACT:

	In the 'legacy mode' (i.e '-o newinstance' option is never specified),
	there should be no change in behavior.
	
	In multi-instance mode (i.e '-o newinstance mount option is specified
	at least once) following user-space issues should be noted.
	
	1. The multi-instance mounts have a 'ptmx' node created/destroyed
	   automatically when devpts is mounted/unmounted. The legacy-mode
	   mounts do not have this node.

	2. To effectively use the multi-instance mode, applications/libraries
	    should, open "/dev/pts/ptmx" instead of "/dev/ptmx" but obviously
	    this would fail in the legacy mode.
	    
	    To work in either legacy or multi-instance mode, applications
	    could replace:

		master_fd = open("/dev/ptmx", flags);

	    with

		if (access("/dev/pts/ptmx", A_OK))
			master_fd = open("/dev/pts/ptmx", flags);
		else
			master_fd = open("/dev/ptmx", flags);

	   To maintain backward compatibility, administrators or startup
	   scripts can "redirect" open of /dev/ptmx to /dev/pts/ptmx in
	   multi-instance mode using a bind mount.

		mount -t devpts -o newinstance devpts /dev/pts
		mount -o bind /dev/pts/ptmx /dev/ptmx

	3. A multi-instance mount that is not accompanied by above bind mount
	   would result in an unusable/unreachable tty to applications that
	   open "/dev/ptmx". i.e

	   	mount -t devpts -o newinstance lxcpts /dev/pts

	   followed by:

		open("/dev/ptmx")

	    would create a pty, say /dev/pts/7, in the initial kernel mount.
	    But /dev/pts/7 would be invisible in the new mount.

	    TODO:

		- We need to document this clearly somewhere (or can the kernel
		  automatically establish the bind mount).
	   
	4. The permissions for "/dev/pts/ptmx" node should be specified when
	   mounting /dev/pts, using the '-o ptmxmode=%o' mount option (default
	   is 0666). 
	   
	   	mount -t devpts -o newinstance -o ptmxmode=0644 devpts /dev/pts

	   The permissions can be later be changed as usual with 'chmod'.

	   	chmod 666 /dev/pts/ptmx

TODO:

	- Document impact of not bind mounting /dev/pts/ptmx after a 
	  multi-instance mount

	- Can we print some friendly message either from kernel or in common
	  user-space commands when this disconnect happens ?

	  
Implementation note:

	See comments in new get_sb_ref() function in fs/super.c on why
	get_sb_single() cannot be directly used.

Changelog[v3]:
	- Rename new mount option to 'newinstance'

	- Create ptmx nodes only in 'newinstance' mounts

	- Bugfix: parse_mount_options() modifies @data but since we need to
	  parse the @data twice (once in devpts_get_sb() and once during
	  do_remount_sb()), parse a local copy of @data in devpts_get_sb().
	  (restructured code in devpts_get_sb() to fix this)

Changelog[v2]:
	- Support both single-mount and multiple-mount semantics and
	  provide '-onewmnt' option to select the semantics.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

---
 fs/devpts/inode.c  |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/super.c         |   43 ++++++++++++++
 include/linux/fs.h |    2 
 3 files changed, 203 insertions(+), 4 deletions(-)

Index: linux-2.6.27-rc3-tty/fs/devpts/inode.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/devpts/inode.c	2008-09-03 21:34:36.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/devpts/inode.c	2008-09-03 21:53:59.000000000 -0700
@@ -42,10 +42,11 @@ struct pts_mount_opts {
 	gid_t   gid;
 	umode_t mode;
 	umode_t ptmxmode;
+	int newinstance;
 };
 
 enum {
-	Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode,
+	Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode, Opt_newinstance,
 	Opt_err
 };
 
@@ -54,6 +55,7 @@ static match_table_t tokens = {
 	{Opt_gid, "gid=%u"},
 	{Opt_mode, "mode=%o"},
 	{Opt_ptmxmode, "ptmxmode=%o"},
+	{Opt_newinstance, "newinstance"},
 	{Opt_err, NULL}
 };
 
@@ -85,6 +87,7 @@ static int parse_mount_options(char *dat
 	opts->gid     = 0;
 	opts->mode    = DEVPTS_DEFAULT_MODE;
 	opts->ptmxmode = DEVPTS_DEFAULT_PTMX_MODE;
+	opts->newinstance = 0;
 
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
@@ -118,6 +121,9 @@ static int parse_mount_options(char *dat
 				return -EINVAL;
 			opts->ptmxmode = option & S_IALLUGO;
 			break;
+		case Opt_newinstance:
+			opts->newinstance = 1;
+			break;
 		default:
 			printk(KERN_ERR "devpts: called with bogus options\n");
 			return -EINVAL;
@@ -127,6 +133,53 @@ static int parse_mount_options(char *dat
 	return 0;
 }
 
+/*
+ * Safely parse the mount options in @data and update @opts.
+ *
+ * devpts ends up parsing options several times during mount, due to the
+ * two modes of operation it supports.
+ *
+ * The initial mount of single-instance mode parses options twwo times:
+ * 	- in devpts_get_sb() to determine the type of mount
+ * 	- in devpts_remount (when get_sb_single() calls do_remount_sb())
+ *
+ * Subsequent mounts in single-instance mode parses options two times:
+ * 	- in devpts_get_sb() to determine type of mount
+ * 	- in devpts_remount (when get_sb_single() calls do_remount_sb())
+ *
+ * Multi-instance mount parses options two times:
+ * 	- in devpts_get_sb() to determine type of mount
+ * 	- in new_pts_mount() to record options
+ *
+ * Since the locations that we parse the options can occur from more than
+ * one place, there does not seem to be a way to parse once and save/use
+ * the results.
+ *
+ * As if this was not messy enough, parsing of options modifies the @data
+ * making subsequent parsing incorrect. Hence the safe_parse_mount_options().
+ *
+ * Return: 0 On success, -errno on error
+ */
+static int safe_parse_mount_options(void *data, struct pts_mount_opts *opts)
+{
+	int rc;
+	void *datacp;
+
+	if (!data)
+		return 0;
+
+	/* Use kstrdup() ?  */
+	datacp = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!datacp)
+		return -ENOMEM;
+
+	memcpy(datacp, data, PAGE_SIZE);
+	rc = parse_mount_options((char *)datacp, opts);
+	kfree(datacp);
+
+	return rc;
+}
+
 static int devpts_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
@@ -145,7 +198,10 @@ static int devpts_show_options(struct se
 	if (opts->setgid)
 		seq_printf(seq, ",gid=%u", opts->gid);
 	seq_printf(seq, ",mode=%03o", opts->mode);
-	seq_printf(seq, ",ptmxmode=%03o", opts->ptmxmode);
+	if (opts->newinstance) {
+		seq_printf(seq, ",ptmxmode=%03o", opts->ptmxmode);
+		seq_printf(seq, ",newinstance");
+	}
 
 	return 0;
 }
@@ -259,10 +315,107 @@ int mknod_ptmx(struct super_block *sb)
 	return 0;
 }
 
+/*
+ * Mount or remount the initial kernel mount of devpts. This type of
+ * mount maintains the legacy, single-instance semantics.
+ */
+static int init_pts_mount(struct file_system_type *fs_type, int flags,
+		void *data, struct vfsmount *mnt)
+{
+	int err;
+
+	if (!devpts_mnt) {
+		err = get_sb_single(fs_type, flags, data, devpts_fill_super,
+				mnt);
+		if (!err)
+			devpts_mnt = mnt;
+
+		return err;
+	}
+
+	return get_sb_ref(devpts_mnt->mnt_sb, flags, data, mnt);
+}
+
+/*
+ * Mount a new (private) instance of devpts. This is selected via
+ * the '-o newinstance' mount option and the PTYs created in this
+ * instance are independent of the PTYs in other devpts instances.
+ *
+ * This type of mount is used in containers to provide isolated PTYs.
+ */
+static int new_pts_mount(struct file_system_type *fs_type, int flags,
+		void *data, struct vfsmount *mnt)
+{
+	int err;
+	struct pts_fs_info *fsi;
+	struct pts_mount_opts *opts;
+
+	printk(KERN_NOTICE "devpts: newinstance mount\n");
+
+	err = get_sb_nodev(fs_type, flags, data, devpts_fill_super, mnt);
+	if (err)
+		return err;
+
+	/*
+	 * Parse mount options here rather than in devpts_fill_super()
+	 * to avoid unnecessary repetition of the parsing in single-
+	 * instance mode.
+	 */
+	fsi = DEVPTS_SB(mnt->mnt_sb);
+	opts = &fsi->mount_opts;
+
+	err = parse_mount_options(data, opts);
+	if (err)
+		goto fail;
+
+	err = mknod_ptmx(mnt->mnt_sb);
+	if (err)
+		goto fail;
+
+	return 0;
+
+fail:
+	dput(mnt->mnt_sb->s_root);
+	deactivate_super(mnt->mnt_sb);
+	return err;
+}
+
+/*
+ * Check if 'newinstance' mount option was specified in @data.
+ *
+ * Return: -errno  	on error (eg: invalid mount options specified)
+ * 	 : 1 		if 'newinstance' mount option was specified
+ * 	 : 0 		if 'newinstance' mount option was NOT specified
+ */
+static int is_new_instance_mount(void *data)
+{
+	int rc;
+	struct pts_mount_opts opts;
+
+	if (!data)
+		return 0;
+
+	rc = safe_parse_mount_options(data, &opts);
+	if (!rc)
+		rc = opts.newinstance;
+
+	return rc;
+}
+
+
 static int devpts_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
-	return get_sb_single(fs_type, flags, data, devpts_fill_super, mnt);
+	int new;
+
+	new = is_new_instance_mount(data);
+	if (new < 0)
+		return new;
+
+	if (new)
+		return new_pts_mount(fs_type, flags, data, mnt);
+
+	return init_pts_mount(fs_type, flags, data, mnt);
 }
 
 
@@ -393,8 +546,9 @@ void devpts_pty_kill(struct tty_struct *
 	if (dentry && !IS_ERR(dentry)) {
 		inode->i_nlink--;
 		d_delete(dentry);
-		dput(dentry);
+		dput(dentry);		// d_lookup in devpts_pty_new
 	}
+	dput(dentry);			// d_find_alias above
 
 	mutex_unlock(&root->d_inode->i_mutex);
 }
Index: linux-2.6.27-rc3-tty/fs/super.c
===================================================================
--- linux-2.6.27-rc3-tty.orig/fs/super.c	2008-09-03 21:28:11.000000000 -0700
+++ linux-2.6.27-rc3-tty/fs/super.c	2008-09-03 21:59:42.000000000 -0700
@@ -883,6 +883,49 @@ int get_sb_single(struct file_system_typ
 
 EXPORT_SYMBOL(get_sb_single);
 
+int get_sb_ref(struct super_block *sb, int flags, void *data,
+		struct vfsmount *mnt)
+{
+	int err;
+
+	/*
+	 * UGLY:
+	 *
+	 * This is needed to support multiple mounts in devpts while
+	 * preserving backward compatibility of the current 'single-mount'
+	 * semantics.
+	 *
+	 * devpts cannot simply use get_sb_single(), bc get_sb_single() or
+	 * more specifically, sget() finds the most recent mount of devpts.
+	 * But that recent mount may not the be initial kernel mount (user
+	 * may have mounted with the '-onewinstance' option since the initial
+	 * mount and get_sb_single() would pick that super-block).
+	 *
+	 * Assuming caller has a valid/initialized sb, unroll essentials of
+	 * get_sb_single() here.
+	 */
+	spin_lock(&sb_lock);
+
+	if (!grab_super(sb)) {
+		/*
+		 * TODO: anymore cleanup ?
+		 */
+		return -EAGAIN;
+	}
+
+	err = do_remount_sb(sb, flags, data, 0);
+	if (err) {
+		/*
+		 * (don't deactivate_super() here - its from initial pts mount)
+		 *
+		 * TODO: anymore cleanup ?
+		 */
+		up_write(&sb->s_umount);
+		return err;
+	}
+	return simple_set_mnt(mnt, sb);
+}
+
 struct vfsmount *
 vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void *data)
 {
Index: linux-2.6.27-rc3-tty/include/linux/fs.h
===================================================================
--- linux-2.6.27-rc3-tty.orig/include/linux/fs.h	2008-09-03 21:28:11.000000000 -0700
+++ linux-2.6.27-rc3-tty/include/linux/fs.h	2008-09-03 21:34:47.000000000 -0700
@@ -1516,6 +1516,8 @@ extern int get_sb_nodev(struct file_syst
 	int flags, void *data,
 	int (*fill_super)(struct super_block *, void *, int),
 	struct vfsmount *mnt);
+extern int get_sb_ref(struct super_block *sb, int flags, void *data,
+	struct vfsmount *mnt);
 void generic_shutdown_super(struct super_block *sb);
 void kill_block_super(struct super_block *sb);
 void kill_anon_super(struct super_block *sb);

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> 
> 	2. To effectively use the multi-instance mode, applications/libraries
> 	    should, open "/dev/pts/ptmx" instead of "/dev/ptmx" but obviously
> 	    this would fail in the legacy mode.
> 	

NOT SO!

/dev/ptmx is required by Unix98 (which is arguably obsolete, but still.) 
  Applications SHOULD NOT try to open /dev/pts/pmtx.  This should be 
considered strictly an internal implementation detail.

Applications should use posix_openpt(), openpty() or forkpty(); 
libraries should use /dev/ptmx.

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

H. Peter Anvin [hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org] wrote:
> sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
>> 	2. To effectively use the multi-instance mode, applications/libraries
>> 	    should, open "/dev/pts/ptmx" instead of "/dev/ptmx" but obviously
>> 	    this would fail in the legacy mode.
>> 	
>
> NOT SO!
>
> /dev/ptmx is required by Unix98 (which is arguably obsolete, but still.)  
> Applications SHOULD NOT try to open /dev/pts/pmtx.  This should be 
> considered strictly an internal implementation detail.

Ah, ok.  Well, I will remove that para from the patch description.

If the -o newinstance is NOT followed by the bind mount, ptys won't
work and would be nice if we can print a useful message when opening
/dev/ptmx.

>
> Applications should use posix_openpt(), openpty() or forkpty(); libraries 
> should use /dev/ptmx.
>
> 	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> 
> Ah, ok.  Well, I will remove that para from the patch description.
> 
> If the -o newinstance is NOT followed by the bind mount, ptys won't
> work and would be nice if we can print a useful message when opening
> /dev/ptmx.
> 

We can't, really, because it will open the global ptmx.  This is an 
unfortunate side effect of the backwards-compatibility code.

This is also why I don't like the bind mount; the symlink option has the 
nice property that f*ckups are more obvious.

The non-legacy option should be as follows, IMNSHO:

- ALL mounts of devpts use -o newinstance;
- /dev/ptmx -> pts/ptmx symlink.

	-hpa

Re: [PATCH 0/11][v3]: Enable multiple mounts of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org [sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| 
| TODO:
| 	- Do we need a '-o ptmxuid' and '-o ptmxgid' options as well ?
| 	- Add a config option to enable multiple-mounts of devpts.
| 	- (Sometime in future) Remove even initial kernel mount of devpts 
| 	- Any other good test suites to test this (besides LTP, sshd).

- Update mount(8) man page with new mount options.
- Add a section to pts(4) mentioning the multiple-instance mode and the
  bind-mount.

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[Alan Cox]

O> We can't, really, because it will open the global ptmx.  This is an 
> unfortunate side effect of the backwards-compatibility code.
> 
> This is also why I don't like the bind mount; the symlink option has the 
> nice property that f*ckups are more obvious.

It's asking for trouble with existing systems and users that
upgrade. /dev/ptmx should remain a proper device file for the non
container case.

Should /dev/ptmx give you a node in the 'master' pty namespace or a node
in your current containers pty namespace ?

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

Alan Cox wrote:
> O> We can't, really, because it will open the global ptmx.  This is an 
>> unfortunate side effect of the backwards-compatibility code.
>>
>> This is also why I don't like the bind mount; the symlink option has the 
>> nice property that f*ckups are more obvious.
> 
> It's asking for trouble with existing systems and users that
> upgrade. /dev/ptmx should remain a proper device file for the non
> container case.

I did say that as being the desired *eventual* goal.

> Should /dev/ptmx give you a node in the 'master' pty namespace or a node
> in your current containers pty namespace ?

Well, since there is no "current containers pty namespace" per se, it 
will give you a node in the default (initial) pty namespace unless the 
bind mount is set up.

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

H. Peter Anvin [hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org] wrote:
> Alan Cox wrote:
>> O> We can't, really, because it will open the global ptmx.  This is an 
>>> unfortunate side effect of the backwards-compatibility code.
>>>
>>> This is also why I don't like the bind mount; the symlink option has the 
>>> nice property that f*ckups are more obvious.
>> It's asking for trouble with existing systems and users that
>> upgrade. /dev/ptmx should remain a proper device file for the non
>> container case.
>
> I did say that as being the desired *eventual* goal.
>
>> Should /dev/ptmx give you a node in the 'master' pty namespace or a node
>> in your current containers pty namespace ?
>
> Well, since there is no "current containers pty namespace" per se, it will 
> give you a node in the default (initial) pty namespace unless the bind
> mount is set up.

But that node will not be accessible if there is a newinstance mount
without the bind mount ? IOW

	1. mount -t devpts -o newinstance lxcpts /dev/pts
	2. mount -o bind /dev/pts/ptmx /dev/ptmx

If both #1 and #2 or neither happen there is no problem.

If #1 is NOT followed by #2, ptys break in new namespace.

An open of /dev/ptmx in this case will allocate a pty in the
initial namespace, but since #1 is complete, we lookup the
pty (/dev/pts/7) in the new namespace and fail.

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> 
> But that node will not be accessible if there is a newinstance mount
> without the bind mount ? IOW
> 
> 	1. mount -t devpts -o newinstance lxcpts /dev/pts
> 	2. mount -o bind /dev/pts/ptmx /dev/ptmx
> 
> If both #1 and #2 or neither happen there is no problem.
> 
> If #1 is NOT followed by #2, ptys break in new namespace.
> 
> An open of /dev/ptmx in this case will allocate a pty in the
> initial namespace, but since #1 is complete, we lookup the
> pty (/dev/pts/7) in the new namespace and fail.
> 

That is correct.

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

H. Peter Anvin [hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org] wrote:
> sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
>> But that node will not be accessible if there is a newinstance mount
>> without the bind mount ? IOW
>> 	1. mount -t devpts -o newinstance lxcpts /dev/pts
>> 	2. mount -o bind /dev/pts/ptmx /dev/ptmx
>> If both #1 and #2 or neither happen there is no problem.
>> If #1 is NOT followed by #2, ptys break in new namespace.
>> An open of /dev/ptmx in this case will allocate a pty in the
>> initial namespace, but since #1 is complete, we lookup the
>> pty (/dev/pts/7) in the new namespace and fail.
>
> That is correct.

So afaics, we don't have any issues when operating only in one mode
(single-instance or multi-instance).

When both modes are used simultaneously, we have following options:

1. Let container-startup deal with it i.e use above bind-mount approach
   or, as Serge mentioned, have containers chroot and make ptmx->pts/ptmx
   symlink or another option ?

2. Have the ptmx-node even in the initial mount and a "permanent" ptmx
   symlink -  Did we fully rule it out :-)

3. Choose #2 with a (yet-another) config token. Not sure if it adds
   value or further complicates the matrix.

Both #1 and #2 have their pros/cons.  Long term, one advantage I see with #2
is that we don't force container-scripts do something now that they can/should
potentially undo later if we ever want to remove the single-instance semantics.

Does presence of /dev/pts/ptmx in single-instance case break userspace ?
If it only surprises, will adding notes to pts(4) man page help ?

Or are there other options ?

Thanks,

Suka

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:

> 
> When both modes are used simultaneously, we have following options:
> 
> 1. Let container-startup deal with it i.e use above bind-mount approach
>    or, as Serge mentioned, have containers chroot and make ptmx->pts/ptmx
>    symlink or another option ?
> 
> 2. Have the ptmx-node even in the initial mount and a "permanent" ptmx
>    symlink -  Did we fully rule it out :-)
> 
> 3. Choose #2 with a (yet-another) config token. Not sure if it adds
>    value or further complicates the matrix.
> 
> Both #1 and #2 have their pros/cons.  Long term, one advantage I see with #2
> is that we don't force container-scripts do something now that they can/should
> potentially undo later if we ever want to remove the single-instance semantics.
> 
> Does presence of /dev/pts/ptmx in single-instance case break userspace ?
> If it only surprises, will adding notes to pts(4) man page help ?
> 

Well, userspaces which implement the #2 option should add the 
newinstance mount option to ALL mounts of devpts, including the first 
one.  That way the "default" pts instance is never actually exposed.

Container scripts which need to work in both modes can trivially 
determine if they need to do the bind-mount, simply by seeing if 
/dev/ptmx is already a symlink.

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[Alan Cox]

> Does presence of /dev/pts/ptmx in single-instance case break userspace ?

It changes the permssion rules and subverts any permissions and security
labels applied to the current node.

If it was there and defaulted to no permission I doubt anything would
care - ie presence is not the problem, rights management is.

Alan

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

Alan Cox wrote:
>> Does presence of /dev/pts/ptmx in single-instance case break userspace ?
> 
> It changes the permssion rules and subverts any permissions and security
> labels applied to the current node.
> 
> If it was there and defaulted to no permission I doubt anything would
> care - ie presence is not the problem, rights management is.

It would be easy enough to have it default to mode 000 unless otherwise 
specified.  For the default instance it is important that a remount can 
update the permissions (since the original mount will be the kernel 
version), but that's pretty straightforward.

That might be the best option?

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

H. Peter Anvin [hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org] wrote:
> Alan Cox wrote:
>>> Does presence of /dev/pts/ptmx in single-instance case break userspace ?
>> It changes the permssion rules and subverts any permissions and security
>> labels applied to the current node.
>> If it was there and defaulted to no permission I doubt anything would
>> care - ie presence is not the problem, rights management is.
>
> It would be easy enough to have it default to mode 000 unless otherwise 
> specified.  For the default instance it is important that a remount can 
> update the permissions (since the original mount will be the kernel 
> version), but that's pretty straightforward.

Agree in general. Not sure if you are implying remount is necessary just
to change permissions of pts/ptmx. Why not "chmod 0666 /dev/pts/ptmx" ?
The remount changes the 'ptmxmode' setting, but since the node exists,
the 'ptmxmode' setting is never used again and we need to chmod.

> That might be the best option?

For containers or multi-instance mode, I agree.

In mixed mode, one observation is if /dev/ptmx is changed to symlink, regular
(not container) startup scripts must chmod /dev/pts/ptmx on _every_ boot.

ptmx node in multi-instance mounts continue to get PTMX_DEFAULT_MODE
permissions (not 000) right ? (unless -o ptmxmode is specified)

Yes, I think its a good option.

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[H. Peter Anvin]

sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> Agree in general. Not sure if you are implying remount is necessary just
> to change permissions of pts/ptmx. Why not "chmod 0666 /dev/pts/ptmx" ?
> The remount changes the 'ptmxmode' setting, but since the node exists,
> the 'ptmxmode' setting is never used again and we need to chmod.

A chmod requires bigger changes to existing scripts than an option which 
can be set in /etc/fstab.

> ptmx node in multi-instance mounts continue to get PTMX_DEFAULT_MODE
> permissions (not 000) right ? (unless -o ptmxmode is specified)

It's probably easier to always default it to zero and expect that the 
mode is set explicitly.

	-hpa

Re: [PATCH 11/11][v3]: Enable multiple instances of devpts

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA]

H. Peter Anvin [hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org] wrote:
> sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
>> Agree in general. Not sure if you are implying remount is necessary just
>> to change permissions of pts/ptmx. Why not "chmod 0666 /dev/pts/ptmx" ?
>> The remount changes the 'ptmxmode' setting, but since the node exists,
>> the 'ptmxmode' setting is never used again and we need to chmod.
>
> A chmod requires bigger changes to existing scripts than an option which 
> can be set in /etc/fstab.

Ok. From implementation pov, we can cache the ptmx dentry in s_fs_info
and use it during remount to change the permissions. We could lookup
ptmx during remount, but caching is simpler ?

>
>> ptmx node in multi-instance mounts continue to get PTMX_DEFAULT_MODE
>> permissions (not 000) right ? (unless -o ptmxmode is specified)
>
> It's probably easier to always default it to zero and expect that the mode 
> is set explicitly.

Ok.

BTW, I have added CONFIG_DEVPTS_MULTIPLE_INSTANCES and moved the usage
info from 11/11 to Documentation/fs/devpts.txt.

Will make above ptmx changes, port to recent ttydev tree and send updated
patchset in a couple of days.

Thanks,

suka