Skip to contentSkip to conversation

RFC: deprecating/removing the legacy mode of devpts

DISCUSSION H. Peter Anvin 7 participants 34 messages Apr 7, 2012 – Apr 12, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Currently, devpts supports a legacy mode where only one instance exists.
For backwards compatibility, this is also the default mode, but the
right thing should be to migrate everyone over to the "newinstance" scheme.

However, it is increasingly clear that that is not happening; neither the distros nor udev support this by default at this point.

I think it might be time we set a sunset date for the legacy mode, probably around kernel 3.9 at this point.

The big change is that /dev/ptmx instead of being a device node should be a symlink to /dev/pts/ptmx, which is then either chmodded to the proper permissions, or given proper permissions via the ptmxmode mount options to devpts. The latter is probably preferable from a technical point of view since it doesn't create a "gap".

As such, I would like to suggest the following:

1. For 3.5, add this to feature-removal.txt.
2. For 3.6, remove CONFIG_DEVPTS_MULTIPLE_INSTANCES as a configuration option, and printk a warning if devpts is mounted without the "newinstance" option.
3. For 3.9 remove the legacy support including support for a ptmx node outside the devpts filesystem.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#1 lore raw
Quoted 6 lines · On Sat, 07 Apr 2012 11:36:26 -0700 "H. Peter Anvin" <[email protected]> wrote:

Currently, devpts supports a legacy mode where only one instance exists.
For backwards compatibility, this is also the default mode, but the
right thing should be to migrate everyone over to the "newinstance" scheme.

However, it is increasingly clear that that is not happening; neither the distros nor udev support this by default at this point.

Well maybe thats because the legacy mode works perfectly well for most purposes and does what they want. The "non-legacy" mode is really not "non-legacy" its "special case extra funkiness".

With my tty hat on I'd prefer to keep it indefinitely if need be. It's hardly a maintainability issue or a hot problem. We've got much bigger turds to juggle than that.

I don't see why we need an agendum to force the users into making changes that really don't matter. I mean we still support BSD ptys 8)

Alan

#2 lore raw

The problem is that the lack of migration at the root causes real problems for the people who need the extra funkiness, and unlike BSD ttys it's not an application-level change at all.

Quoted 25 lines · Alan Cox
Quoted 1 line · On Sat, 07 Apr 2012 11:36:26 -0700 "H. Peter Anvin" <[email protected]> wrote:

Currently, devpts supports a legacy mode where only one instance

exists.

Quoted 2 lines

For backwards compatibility, this is also the default mode, but the
right thing should be to migrate everyone over to the "newinstance"

scheme.

Quoted 3 lines

However, it is increasingly clear that that is not happening; neither the distros nor udev support this by default at this point.

Well maybe thats because the legacy mode works perfectly well for most purposes and does what they want. The "non-legacy" mode is really not "non-legacy" its "special case extra funkiness".

With my tty hat on I'd prefer to keep it indefinitely if need be. It's hardly a maintainability issue or a hot problem. We've got much bigger turds to juggle than that.

I don't see why we need an agendum to force the users into making changes
that really don't matter. I mean we still support BSD ptys 8)

Alan

Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#25 lore raw
In reply to Alan Cox

Even "doing it manually" gets hard because udev gets in the way.

Quoted 25 lines · Alan Cox
Quoted 1 line · On Sat, 07 Apr 2012 11:36:26 -0700 "H. Peter Anvin" <[email protected]> wrote:

Currently, devpts supports a legacy mode where only one instance

exists.

Quoted 2 lines

For backwards compatibility, this is also the default mode, but the
right thing should be to migrate everyone over to the "newinstance"

scheme.

Quoted 3 lines

However, it is increasingly clear that that is not happening; neither the distros nor udev support this by default at this point.

Well maybe thats because the legacy mode works perfectly well for most purposes and does what they want. The "non-legacy" mode is really not "non-legacy" its "special case extra funkiness".

With my tty hat on I'd prefer to keep it indefinitely if need be. It's hardly a maintainability issue or a hot problem. We've got much bigger turds to juggle than that.

I don't see why we need an agendum to force the users into making changes
that really don't matter. I mean we still support BSD ptys 8)

Alan

Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#3 lore raw
In reply to H. Peter Anvin
Quoted 3 lines · H. Peter Anvin

The problem is that the lack of migration at the root causes real problems for the people who need the extra funkiness, and unlike BSD ttys it's not an application-level change at all.

Could you explain what the issues are? I haven't been following devpts all that carefully.

Thanks,

- Ted

#4 lore raw
Quoted 8 lines · Ted Ts'o
Quoted 3 lines · H. Peter Anvin

The problem is that the lack of migration at the root causes real problems for the people who need the extra funkiness, and unlike BSD ttys it's not an application-level change at all.

Could you explain what the issues are? I haven't been following devpts all that carefully.

The issue is that to make the "ptys are private to an instance of the devpts filesystem", /dev/ptmx has to live inside the /dev/pts filesystem. This was traditionally not the case. To avoid breaking everything all at once, we have a legacy mode which supports the "all devpts instances are the same" (effectively bind mounts) and which support a /dev/ptmx outside /dev/pts.

The problem is that anyone who wants to take advantage of the new functionality has to make sure *all* instances of devpts work with the new protocol. This means modifying your distro to:

1. Add "newinstance" and "ptmxmode" to /etc/fstab [easy]
2. Make /dev/ptmx a symlink to /dev/pts/ptmx.

2 would be easy if it wasn't for udev, which makes it very hard. It is not reasonable for udev to support both modes, so the *only* mode that is reasonable for it to support is the new mode. However, it is increasingly obvious that if we don't force it, this will never happen.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#5 lore raw
Quoted 21 lines · H. Peter Anvin
Quoted 8 lines · Ted Ts'o
Quoted 3 lines · H. Peter Anvin

The problem is that the lack of migration at the root causes real problems for the people who need the extra funkiness, and unlike BSD ttys it's not an application-level change at all.

Could you explain what the issues are?  I haven't been following devpts all that carefully.

The issue is that to make the "ptys are private to an instance of the devpts filesystem", /dev/ptmx has to live inside the /dev/pts filesystem.  This was traditionally not the case.  To avoid breaking everything all at once, we have a legacy mode which supports the "all devpts instances are the same" (effectively bind mounts) and which support a /dev/ptmx outside /dev/pts.

The problem is that anyone who wants to take advantage of the new functionality has to make sure *all* instances of devpts work with the new protocol.  This means modifying your distro to:

Quoted 7 lines

1. Add "newinstance" and "ptmxmode" to /etc/fstab [easy]
2. Make /dev/ptmx a symlink to /dev/pts/ptmx.

2 would be easy if it wasn't for udev, which makes it very hard.  It is not reasonable for udev to support both modes, so the *only* mode that is reasonable for it to support is the new mode.  However, it is increasingly obvious that if we don't force it, this will never happen.

I don't think it has much to do with udev, it follows 100% instructions from the kernel, and it does not create the device node that is in the way here.

Udev does not name any device in the system since a long time. Since quite a while it has not even the code to do mknod() and requires devtmpfs. The device node part of udev these day is limited to manage device node permissions and creating additional symlinks. All device node creation happens inside the kernel itself - where it belongs - and not in userspace.

If the default behaviour of /dev/pts/* should be changed, the kernel should be changed to support the multi-instance mode right away without involving userspace. We better do not require userspace to gain any knowledge about such stuff. I'm confident, that we should not add more, or require to support multiple alternative ways of handling kernel internals in userspace.

So, I think we either remove the '/sys/class/tty/ptmx' device from the system, and let the devpts code create the symlink in the 'devtmpfs' filesystem, or alternatively the '/sys/class/tty/ptmx' device supports the multi-instance mode itself, instead of requiring a symlink. Such stuff belongs entirely into the kernel these day. Anything else seems to just ask for trouble.

Kay

#24 lore raw
In reply to H. Peter Anvin
Quoted 4 lines

2 would be easy if it wasn't for udev, which makes it very hard. It is not reasonable for udev to support both modes, so the *only* mode that is reasonable for it to support is the new mode. However, it is increasingly obvious that if we don't force it, this will never happen.

If enough people care about it then udev will get fixed. Using the kernel as a large hammer to try and force one worldview onto people who couldn't give a flying.. isn't the right answer here.

Get the distros to switch mode by asking them and/or by sending them changes for new releases.

Alan

#6 lore raw
In reply to Kay Sievers
Quoted 19 lines · Kay Sievers

I don't think it has much to do with udev, it follows 100% instructions from the kernel, and it does not create the device node that is in the way here.

Udev does not name any device in the system since a long time. Since quite a while it has not even the code to do mknod() and requires devtmpfs. The device node part of udev these day is limited to manage device node permissions and creating additional symlinks. All device node creation happens inside the kernel itself - where it belongs - and not in userspace.

If the default behaviour of /dev/pts/* should be changed, the kernel should be changed to support the multi-instance mode right away without involving userspace. We better do not require userspace to gain any knowledge about such stuff. I'm confident, that we should not add more, or require to support multiple alternative ways of handling kernel internals in userspace.

Quoted 7 lines

So, I think we either remove the '/sys/class/tty/ptmx' device from the system, and let the devpts code create the symlink in the 'devtmpfs' filesystem, or alternatively the '/sys/class/tty/ptmx' device supports the multi-instance mode itself, instead of requiring a symlink. Such stuff belongs entirely into the kernel these day. Anything else seems to just ask for trouble.

OK, this seems very reasonable, and something that could (and probably *has to*) be done as an atomic change... maybe.

There is no way for /dev/ptmx to support the multiinstance mode since it is located outside any devpts filesystem; it *has* to be inside the devpts filesystem in order to function... if that wasn't the case this whole thing would have been trivial from the get-go.

Greg, do you have any insights in what would have to be necessary mechanics to make this.

-hpa

#26 lore raw
In reply to H. Peter Anvin
Quoted 18 lines · H. Peter Anvin

Currently, devpts supports a legacy mode where only one instance exists.
For backwards compatibility, this is also the default mode, but the
right thing should be to migrate everyone over to the "newinstance" scheme.

However, it is increasingly clear that that is not happening; neither the distros nor udev support this by default at this point.

I think it might be time we set a sunset date for the legacy mode, probably around kernel 3.9 at this point.

The big change is that /dev/ptmx instead of being a device node should be a symlink to /dev/pts/ptmx, which is then either chmodded to the proper permissions, or given proper permissions via the ptmxmode mount options to devpts. The latter is probably preferable from a technical point of view since it doesn't create a "gap".

As such, I would like to suggest the following:

Quoted 9 lines

1. For 3.5, add this to feature-removal.txt.
2. For 3.6, remove CONFIG_DEVPTS_MULTIPLE_INSTANCES as a configuration option, and printk a warning if devpts is mounted without the "newinstance" option.
3. For 3.9 remove the legacy support including support for a ptmx node outside the devpts filesystem.

-hpa

I think we should give the way to test compatibility as early as possible, so:
2'. For 3.6 add CONFIG_DEVPTS_LEGACY_INSTANCE =y by default, if it =n:
always create new instance (ignore "newinstance") and remove "ptmx" from sysfs.

#7 lore raw
In reply to H. Peter Anvin
Quoted 28 lines · H. Peter Anvin
Quoted 19 lines · Kay Sievers

I don't think it has much to do with udev, it follows 100% instructions from the kernel, and it does not create the device node that is in the way here.

Udev does not name any device in the system since a long time. Since quite a while it has not even the code to do mknod() and requires devtmpfs. The device node part of udev these day is limited to manage device node permissions and creating additional symlinks. All device node creation happens inside the kernel itself - where it belongs - and not in userspace.

If the default behaviour of /dev/pts/* should be changed, the kernel should be changed to support the multi-instance mode right away without involving userspace. We better do not require userspace to gain any knowledge about such stuff. I'm confident, that we should not add more, or require to support multiple alternative ways of handling kernel internals in userspace.

Quoted 7 lines

So, I think we either remove the '/sys/class/tty/ptmx' device from the system, and let the devpts code create the symlink in the 'devtmpfs' filesystem, or alternatively the '/sys/class/tty/ptmx' device supports the multi-instance mode itself, instead of requiring a symlink. Such stuff belongs entirely into the kernel these day. Anything else seems to just ask for trouble.

Quoted 7 lines

OK, this seems very reasonable, and something that could (and probably *has to*) be done as an atomic change... maybe.

There is no way for /dev/ptmx to support the multiinstance mode since it is located outside any devpts filesystem; it *has* to be inside the devpts filesystem in order to function... if that wasn't the case this whole thing would have been trivial from the get-go.

In theory the open() could possibly find out to which namespace it belongs, but I guess the other alternative is much simpler and cleaner anyway.

Quoted 2 lines

Greg, do you have any insights in what would have to be necessary mechanics to make this.

I guess the following:
- remove the 'ptmx' cdev from drivers/tty/pty.c
- add a new devtmpfs_create_link() to drivers/base/devtmpfs.c
- call devtmpfs_create_link() from _init in fs/devpts/inode.c
- change the default of ptxmode=0 to a sane default = 0666
- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

The only difference between multi- and single instantiated would be that you get the same and in the other case a new instance, when you mount. If we remove the CONFIG_ option from the kernel or not, will probably not matter that much any more.

I guess, it would involve zero userspace changes, then unlike your marked-as-[easy] item, it is not easy at all. All initramfs generators would need to be changed to support the new mount options that, and they usually have no config file to add that to. We also do not add kernel filesystems to fstab these days, fstab is almost entirely left to the administrator's customization, and not the distribution's defaults, so it should/can not just be added there, because it is not in there today.

Kay

#27 lore raw
In reply to Konstantin Khlebnikov
Quoted 8 lines
Quoted 8 lines

As such, I would like to suggest the following:

1. For 3.5, add this to feature-removal.txt.
2. For 3.6, remove CONFIG_DEVPTS_MULTIPLE_INSTANCES as a configuration option, and printk a warning if devpts is mounted without the "newinstance" option.
3. For 3.9 remove the legacy support including support for a ptmx node outside the devpts filesystem.

Ok I'm putting my tty maintainers hat on at this point because nobody seems to be listening

I'm NAKking this change, I'm NAKking the feature-removal change.

If you want this to change convince the distributions there is a point in supporting it. This is the *wrong* forum to force a cornercase feature onto a bazillion users, and the wrong way to strongarm maintainers of the relevant user space.

If udev has a problem handling it nicely because of the way the link is generated then *fix that problem* so udev automatically gets both cases right. If need be we can add more messages to udev to tell it what is expected and what devtmpfs should produce.

Alan

#28 lore raw
Quoted 11 lines · Alan Cox

Ok I'm putting my tty maintainers hat on at this point because nobody seems to be listening

I'm NAKking this change, I'm NAKking the feature-removal change.

If you want this to change convince the distributions there is a point in supporting it. This is the *wrong* forum to force a cornercase feature onto a bazillion users, and the wrong way to strongarm maintainers of the relevant user space.

Quoted 5 lines

If udev has a problem handling it nicely because of the way the link is generated then *fix that problem* so udev automatically gets both cases right. If need be we can add more messages to udev to tell it what is expected and what devtmpfs should produce.

What if it turns out we can make udev/devtmpfs make this happen transparently (which it sounds like it might be possible)? Would you be okay with removing the "implicit bind mount" property of the current devpts then?

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#29 lore raw
Quoted 4 lines

What if it turns out we can make udev/devtmpfs make this happen transparently (which it sounds like it might be possible)? Would you be okay with removing the "implicit bind mount" property of the current devpts then?

Once we get to the point where it "just works" I'm quite happy for that to happen unless it turns out there's any large code size/performance hits in having the feature always on. I don't see any.

Alan

#30 lore raw
Quoted 9 lines · Alan Cox
Quoted 4 lines

What if it turns out we can make udev/devtmpfs make this happen transparently (which it sounds like it might be possible)? Would you be okay with removing the "implicit bind mount" property of the current devpts then?

Once we get to the point where it "just works" I'm quite happy for that to happen unless it turns out there's any large code size/performance hits in having the feature always on. I don't see any.

No, it's actually the other way around... it lets us take some pretty nice shortcuts.

-hpa

#31 lore raw
Quoted 6 lines · H. Peter Anvin
Quoted 5 lines · Alan Cox
Quoted 4 lines

What if it turns out we can make udev/devtmpfs make this happen transparently (which it sounds like it might be possible)? Would you be okay with removing the "implicit bind mount" property of the current devpts then?

Quoted 7 lines
Quoted 4 lines

Once we get to the point where it "just works" I'm quite happy for that to happen unless it turns out there's any large code size/performance hits in having the feature always on. I don't see any.

No, it's actually the other way around... it lets us take some pretty nice shortcuts.

If it requires making changes to udev, can we place make sure have a decent amount of time before we remove code from the kernel? I really hate it when people break userspace for the convenience
of kernel programmers. As I recall Linus had something to say
about that….

-- Ted

#32 lore raw
Quoted 13 lines · Theodore Tso
Quoted 11 lines · H. Peter Anvin
Quoted 9 lines · Alan Cox
Quoted 4 lines

What if it turns out we can make udev/devtmpfs make this happen transparently (which it sounds like it might be possible)? Would you be okay with removing the "implicit bind mount" property of the current devpts then?

Once we get to the point where it "just works" I'm quite happy for that to happen unless it turns out there's any large code size/performance hits in having the feature always on. I don't see any.

Quoted 8 lines
Quoted 2 lines

No, it's actually the other way around... it lets us take some pretty nice shortcuts.

If it requires making changes to udev, can we place make sure have a decent amount of time before we remove code from the kernel? I really hate it when people break userspace for the convenience
of kernel programmers. As I recall Linus had something to say
about that….

As mentioned earlier, udev is not involved at all here. It's all in the kernel these days; all device nodes are created by the kernel and not by udev.

The one that is in the way here is created by kernel code, and udev will never gain options or support instructions to delete kernel-created nodes. We refuse to teach udev stupid tricks to work around the kernel's idea of new features and options.

The entire idea of legacy vs. non-legacy for devpts is kind of broken to begin with. Requiring to delete kernel-created device nodes and replacing them with symlinks, like mentioned in the devpts documentation, should never be a job for userspace; it needs to be done properly inside the kernel.

Today, userspace should not be required to fiddle around with such stuff, it just mounts the filesystem like it always did, and nothing else. I does not want to know about any new options in such category, which can work on specific kernels only, and will cause too much trouble to support in the end.

The following patch is just to clean up the confusion about userspace and the unfortunate idea of legacy vs. non-legacy mount options. It is nowhere ready to merge, but it should make is pretty clear, that userspace should be left alone here, and it's a job for the kernel only.

Cheers,
Kay

# ls -l /dev
crw------- 1 root root     10,   1 Apr  9 14:21 psaux
lrwxrwxrwx 1 root root           8 Apr  9 14:20 ptmx -> pts/ptmx
drwxr-xr-x 2 root root           0 Apr  9 14:21 pts
crw-rw-rw- 1 root root      1,   8 Apr  9 14:21 random
# ls -l /dev/pts
total 0
crw--w---- 1 root tty 136, 0 Apr  9 14:22 0
crw--w---- 1 kay  tty 136, 1 Apr  9 14:22 1
crw-rw-rw- 1 root tty   5, 2 Apr  9 14:22 ptmx
---
 drivers/base/devtmpfs.c |   60 ++++++++++++++++++++++++-
 drivers/tty/Kconfig     |   11 ----
 drivers/tty/pty.c       |    3 -
 fs/devpts/inode.c       |  112 +++++++-----------------------------------------
 include/linux/device.h  |    2 
 5 files changed, 76 insertions(+), 112 deletions(-)
drivers/base/devtmpfs.c +56 -4
@@ -40,6 +40,7 @@ static struct req {
 	struct completion done;
 	int err;
 	const char *name;
+	const char *target;
 	umode_t mode;	/* 0 => delete */
 	struct device *dev;
 } *requests;
@@ -84,7 +85,7 @@ int devtmpfs_create_node(struct device *
 	if (!thread)
 		return 0;
 
-	req.mode = 0;
+	memset(&req, 0, sizeof(struct req));
 	req.name = device_get_devnode(dev, &req.mode, &tmp);
 	if (!req.name)
 		return -ENOMEM;
@@ -113,6 +114,31 @@ int devtmpfs_create_node(struct device *
 	return req.err;
 }
 
+int devtmpfs_create_link(const char *name, const char *target)
+{
+	struct req req;
+
+	if (!thread)
+		return 0;
+
+	memset(&req, 0, sizeof(struct req));
+	req.mode = 0666;
+	req.name = name;
+	req.target = target;
+
+	init_completion(&req.done);
+
+	spin_lock(&req_lock);
+	req.next = requests;
+	requests = &req;
+	spin_unlock(&req_lock);
+
+	wake_up_process(thread);
+	wait_for_completion(&req.done);
+
+	return req.err;
+}
+
 int devtmpfs_delete_node(struct device *dev)
 {
 	const char *tmp = NULL;
@@ -121,11 +147,11 @@ int devtmpfs_delete_node(struct device *
 	if (!thread)
 		return 0;
 
+	memset(&req, 0, sizeof(struct req));
 	req.name = device_get_devnode(dev, NULL, &tmp);
 	if (!req.name)
 		return -ENOMEM;
 
-	req.mode = 0;
 	req.dev = dev;
 
 	init_completion(&req.done);
@@ -225,6 +251,28 @@ static int handle_create(const char *nod
 	return err;
 }
 
+static int handle_create_link(const char *name, const char *target)
+{
+	struct dentry *dentry;
+	struct path path;
+	int err;
+
+	dentry = kern_path_create(AT_FDCWD, name, &path, 0);
+	if (dentry == ERR_PTR(-ENOENT)) {
+		create_path(name);
+		dentry = kern_path_create(AT_FDCWD, name, &path, 0);
+	}
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	err = vfs_symlink(path.dentry->d_inode, dentry, target);
+	dput(dentry);
+
+	mutex_unlock(&path.dentry->d_inode->i_mutex);
+	path_put(&path);
+	return err;
+}
+
 static int dev_rmdir(const char *name)
 {
 	struct nameidata nd;
@@ -378,8 +426,11 @@ int devtmpfs_mount(const char *mntdir)
 
 static DECLARE_COMPLETION(setup_done);
 
-static int handle(const char *name, umode_t mode, struct device *dev)
+static int handle(const char *name, umode_t mode,
+		  struct device *dev, const char *target)
 {
+	if (target)
+		return handle_create_link(name, target);
 	if (mode)
 		return handle_create(name, mode, dev);
 	else
@@ -407,7 +458,8 @@ static int devtmpfsd(void *p)
 			spin_unlock(&req_lock);
 			while (req) {
 				struct req *next = req->next;
-				req->err = handle(req->name, req->mode, req->dev);
+				req->err = handle(req->name, req->mode,
+						  req->dev, req->target);
 				complete(&req->done);
 				req = next;
 			}
drivers/tty/Kconfig +0 -11
@@ -109,17 +109,6 @@ config UNIX98_PTYS
 	  All modern Linux systems use the Unix98 ptys.  Say Y unless
 	  you're on an embedded system and want to conserve memory.
 
-config DEVPTS_MULTIPLE_INSTANCES
-	bool "Support multiple instances of devpts"
-	depends on UNIX98_PTYS
-	default n
-	---help---
-	  Enable support for multiple instances of devpts filesystem.
-	  If you want to have isolated PTY namespaces (eg: in containers),
-	  say Y here.  Otherwise, say N. If enabled, each mount of devpts
-	  filesystem with the '-o newinstance' option will create an
-	  independent PTY namespace.
-
 config LEGACY_PTYS
 	bool "Legacy (BSD) PTY support"
 	default y
drivers/tty/pty.c +1 -2
@@ -706,7 +706,7 @@ static void __init unix98_pty_init(void)
 	if (tty_register_driver(pts_driver))
 		panic("Couldn't register Unix98 pts driver");
 
-	/* Now create the /dev/ptmx special device */
+	/* Hook up the ptmx cdev which lives inside the devpts filesystem */
 	tty_default_fops(&ptmx_fops);
 	ptmx_fops.open = ptmx_open;
 
@@ -714,7 +714,6 @@ static void __init unix98_pty_init(void)
 	if (cdev_add(&ptmx_cdev, MKDEV(TTYAUX_MAJOR, 2), 1) ||
 	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 2), 1, "/dev/ptmx") < 0)
 		panic("Couldn't register /dev/ptmx driver\n");
-	device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 2), NULL, "ptmx");
 }
 
 #else
fs/devpts/inode.c +17 -95

233 lines not on this page · raw message

include/linux/device.h +2 -0
@@ -858,10 +858,12 @@ extern void put_device(struct device *de
 extern void wait_for_device_probe(void);
 
 #ifdef CONFIG_DEVTMPFS
+extern int devtmpfs_create_link(const char *name, const char *target);
 extern int devtmpfs_create_node(struct device *dev);
 extern int devtmpfs_delete_node(struct device *dev);
 extern int devtmpfs_mount(const char *mntdir);
 #else
+static int devtmpfs_create_link(const char *name, const char *target) { return 0; }
 static inline int devtmpfs_create_node(struct device *dev) { return 0; }
 static inline int devtmpfs_delete_node(struct device *dev) { return 0; }
 static inline int devtmpfs_mount(const char *mountpoint) { return 0; }
#33 lore raw
Quoted 3 lines · Kay Sievers

As mentioned earlier, udev is not involved at all here. It's all in the kernel these days; all device nodes are created by the kernel and not by udev.

... on the systems that have devtmpfs enabled. And no, "runs castrated udev" is not "all systems out there that might want a new kernel".

#8 lore raw
In reply to Kay Sievers
Quoted 41 lines · Kay Sievers
Quoted 28 lines · H. Peter Anvin
Quoted 19 lines · Kay Sievers

I don't think it has much to do with udev, it follows 100% instructions from the kernel, and it does not create the device node that is in the way here.

Udev does not name any device in the system since a long time. Since quite a while it has not even the code to do mknod() and requires devtmpfs. The device node part of udev these day is limited to manage device node permissions and creating additional symlinks. All device node creation happens inside the kernel itself - where it belongs - and not in userspace.

If the default behaviour of /dev/pts/* should be changed, the kernel should be changed to support the multi-instance mode right away without involving userspace. We better do not require userspace to gain any knowledge about such stuff. I'm confident, that we should not add more, or require to support multiple alternative ways of handling kernel internals in userspace.

Quoted 7 lines

So, I think we either remove the '/sys/class/tty/ptmx' device from the system, and let the devpts code create the symlink in the 'devtmpfs' filesystem, or alternatively the '/sys/class/tty/ptmx' device supports the multi-instance mode itself, instead of requiring a symlink. Such stuff belongs entirely into the kernel these day. Anything else seems to just ask for trouble.

Quoted 7 lines

OK, this seems very reasonable, and something that could (and probably *has to*) be done as an atomic change... maybe.

There is no way for /dev/ptmx to support the multiinstance mode since it is located outside any devpts filesystem; it *has* to be inside the devpts filesystem in order to function... if that wasn't the case this whole thing would have been trivial from the get-go.

In theory the open() could possibly find out to which namespace it belongs, but I guess the other alternative is much simpler and cleaner anyway.

Quoted 8 lines
Quoted 2 lines

Greg, do you have any insights in what would have to be necessary mechanics to make this.

I guess the following:
- remove the 'ptmx' cdev from drivers/tty/pty.c
- add a new devtmpfs_create_link() to drivers/base/devtmpfs.c
- call devtmpfs_create_link() from _init in fs/devpts/inode.c
- change the default of ptxmode=0 to a sane default = 0666

That all seems pretty simple.

Quoted 3 lines

- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Hm, but if we get rid of them, what about tools that expect them to be there? Just silently ignore them?

I'll see if I can knock something up next week...

greg k-h

#9 lore raw
Quoted 10 lines · Greg Kroah-Hartman

That all seems pretty simple.

Quoted 3 lines

- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Hm, but if we get rid of them, what about tools that expect them to be there? Just silently ignore them?

Drop the newinstance mount option on the floor; keep recognizing the ptmxmode mount option but change the default.

-hpa

#10 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Greg Kroah-Hartman

That all seems pretty simple.

Quoted 3 lines

- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Quoted 6 lines
Quoted 3 lines

Hm, but if we get rid of them, what about tools that expect them to be there? Just silently ignore them?

Drop the newinstance mount option on the floor; keep recognizing the ptmxmode mount option but change the default.

Wait, change the default to what?

I think this is going to require a lot of testing :)

greg k-h

#11 lore raw
Quoted 9 lines · Greg Kroah-Hartman
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Greg Kroah-Hartman

That all seems pretty simple.

Quoted 3 lines

  - get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Hm, but if we get rid of them, what about tools that expect them to be there?  Just silently ignore them?

Drop the newinstance mount option on the floor; keep recognizing the ptmxmode mount option but change the default.

Wait, change the default to what?

Of the devpts fs mount option. The 'newinstance' one, which should not have existed in the first place.

Quoted 1 line

I think this is going to require a lot of testing :)

In the example patch I posted and which I ran here, I just removed the entire option. This was just to show that userspace does not want to be taught dirty tricks that need updating of early-boot tools. All such pretty self-contained stuff should be in the kernel itself. Userspace wants to be dumb here, and that worked fine so far with that patch. :)

Kay

#13 lore raw
In reply to Kay Sievers
Quoted 8 lines · Kay Sievers

I guess the following:
- remove the 'ptmx' cdev from drivers/tty/pty.c
- add a new devtmpfs_create_link() to drivers/base/devtmpfs.c
- call devtmpfs_create_link() from _init in fs/devpts/inode.c
- change the default of ptxmode=0 to a sane default = 0666
- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

You do realize that there are people with _static_ /dev out there, don't you? As well as those of us who are not using your latest and greatest udev and not enabling devtmpfs at all. Not all world is Fedora...

#12 lore raw
In reply to Kay Sievers
Quoted 23 lines · Kay Sievers
Quoted 9 lines · Greg Kroah-Hartman
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Greg Kroah-Hartman

That all seems pretty simple.

Quoted 3 lines

  - get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Hm, but if we get rid of them, what about tools that expect them to be there?  Just silently ignore them?

Drop the newinstance mount option on the floor; keep recognizing the ptmxmode mount option but change the default.

Wait, change the default to what?

Of the devpts fs mount option. The 'newinstance' one, which should not have existed in the first place.

Quoted 8 lines
Quoted 1 line

I think this is going to require a lot of testing :)

In the example patch I posted and which I ran here, I just removed the entire option. This was just to show that userspace does not want to be taught dirty tricks that need updating of early-boot tools. All such pretty self-contained stuff should be in the kernel itself. Userspace wants to be dumb here, and that worked fine so far with that patch. :)

I missed your patch, sorry, am catching up on the email thread now...

greg k-h

#14 lore raw
In reply to Al Viro

Just replace /dev/ptmx with a pts/ptmx symlink... something that works already.

Quoted 17 lines · Al Viro
Quoted 6 lines · Kay Sievers

I guess the following:
- remove the 'ptmx' cdev from drivers/tty/pty.c
- add a new devtmpfs_create_link() to drivers/base/devtmpfs.c
- call devtmpfs_create_link() from _init in fs/devpts/inode.c
- change the default of ptxmode=0 to a sane default = 0666
- get rid of the (rather broken) idea of 'legacy' vs 'non-legacy

mode'

Quoted 1 line

mount options in the default setup; I don't think userspace

should

Quoted 1 line

ever be required to fiddle with such stuff

You do realize that there are people with _static_ /dev out there, don't
you? As well as those of us who are not using your latest and greatest udev and not enabling devtmpfs at all. Not all world is Fedora...

Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#15 lore raw
Quoted 1 line · H. Peter Anvin

Just replace /dev/ptmx with a pts/ptmx symlink... something that works already.

It does *not* work for static setups - try to boot stock 3.3 on the root fs with such a symlink and you've got a dangling symlink. Have fun trying to bisect something with repeated jumps back and forth across the point where such change goes in...

It will also do no good on setups like e.g. debian/stable, with stock udev - you need to edit /lib/udev/rules.d/50-udev-default.rules to get that link created, with quite a bit of interesting kludgery to avoid the same kind of crap (i.e. setup breaking when you boot an older kernel).

#19 lore raw
In reply to Al Viro
Quoted 11 lines · Al Viro
Quoted 8 lines · Kay Sievers

I guess the following:
  - remove the 'ptmx' cdev from drivers/tty/pty.c
  - add a new devtmpfs_create_link() to drivers/base/devtmpfs.c
  - call devtmpfs_create_link() from _init in fs/devpts/inode.c
  - change the default of ptxmode=0 to a sane default = 0666
  - get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' mount options in the default setup; I don't think userspace should ever be required to fiddle with such stuff

Quoted 3 lines

You do realize that there are people with _static_ /dev out there, don't you?  As well as those of us who are not using your latest and greatest udev and not enabling devtmpfs at all.  Not all world is Fedora...

Yeah, and? The dev_t still works from the static /dev/ like it didi 10 years ago. All good.

Not all breaks when I talk about it. :)

Kay

#20 lore raw
Quoted 12 lines · Kay Sievers
Quoted 11 lines · Al Viro
Quoted 8 lines · Kay Sievers

I guess the following:
?? - remove the 'ptmx' cdev from drivers/tty/pty.c
?? - add a new devtmpfs_create_link() to drivers/base/devtmpfs.c ?? - call devtmpfs_create_link() from _init in fs/devpts/inode.c ?? - change the default of ptxmode=0 to a sane default = 0666 ?? - get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' ?? ?? mount options in the default setup; I don't think userspace should ?? ?? ever be required to fiddle with such stuff

Quoted 6 lines
Quoted 3 lines

You do realize that there are people with _static_ /dev out there, don't you? ??As well as those of us who are not using your latest and greatest udev and not enabling devtmpfs at all. ??Not all world is Fedora...

Yeah, and? The dev_t still works from the static /dev/ like it didi 10 years ago. All good.

Only if you move that cdev to fs/devpts/inode.c instead of removing it, at which point you get the interesting problems back on other setups...

#16 lore raw
In reply to Al Viro

The symlink support is at least 3 years old, probably more.

Quoted 16 lines · Al Viro
Quoted 1 line · H. Peter Anvin

Just replace /dev/ptmx with a pts/ptmx symlink... something that

works already.

It does *not* work for static setups - try to boot stock 3.3 on the root
fs with such a symlink and you've got a dangling symlink. Have fun trying to bisect something with repeated jumps back and forth across the
point where such change goes in...

It will also do no good on setups like e.g. debian/stable, with stock udev - you need to edit /lib/udev/rules.d/50-udev-default.rules to get that link created, with quite a bit of interesting kludgery to avoid the
same kind of crap (i.e. setup breaking when you boot an older kernel).

Signature · 1 line
Sent from my mobile phone. Please excuse brevity and lack of formatting.
#21 lore raw
In reply to Al Viro
Quoted 17 lines · Al Viro
Quoted 16 lines · Kay Sievers
Quoted 14 lines · Al Viro
Quoted 8 lines · Kay Sievers

I guess the following:
?? - remove the 'ptmx' cdev from drivers/tty/pty.c
?? - add a new devtmpfs_create_link() to drivers/base/devtmpfs.c ?? - call devtmpfs_create_link() from _init in fs/devpts/inode.c ?? - change the default of ptxmode=0 to a sane default = 0666 ?? - get rid of the (rather broken) idea of 'legacy' vs 'non-legacy mode' ?? ?? mount options in the default setup; I don't think userspace should ?? ?? ever be required to fiddle with such stuff

You do realize that there are people with _static_ /dev out there, don't you? ??As well as those of us who are not using your latest and greatest udev and not enabling devtmpfs at all. ??Not all world is Fedora...

Quoted 5 lines
Quoted 2 lines

Yeah, and? The dev_t still works from the static /dev/ like it didi 10 years ago. All good.

Only if you move that cdev to fs/devpts/inode.c instead of removing it, at which point you get the interesting problems back on other setups...

fs/devpts/inode.c uses the very _same_ registered cdev, nobody wants or can remove it.

Kay

#22 lore raw
In reply to Kay Sievers
Quoted 4 lines · Kay Sievers

Yeah, and? The dev_t still works from the static /dev/ like it didi 10 years ago. All good.

That *is* the legacy mode which we'd like to get rid of.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#23 lore raw
Quoted 6 lines · H. Peter Anvin
Quoted 3 lines · Kay Sievers

Yeah, and? The dev_t still works from the static /dev/ like it didi 10 years ago. All good.

That *is* the legacy mode which we'd like to get rid of.

Not sure if that will work. We might need to keep that for a while. And static /dev and old udevs and mdev and whatever will just work as they do today.

But the non-legacy mode compile option can be, as far as I see, still be made working without requiring mount options and without requiring any userspace changes.

What we get rid of then is not the legacy support, but the need to manually switch userspace over. It's just a compile option then, and people with static or ancient stuff do not switch, and everybody else gets switched and will not notice.

Kay

#17 lore raw
In reply to H. Peter Anvin
Quoted 1 line · H. Peter Anvin

The symlink support is at least 3 years old, probably more.

DEVPTS_MULTIPLE_INSTANCES wasn't anywhere near mandatory all that time... Kernel upgrade procedure that starts with "build the old kernel with that option enabled, boot it, replace /dev/ptmx with a symlink, then never try to boot earlier kernel images. Now you can safely upgrade to current kernel" is the recipe for massive self-LARTs (and resulting pitchfork crowds).

#18 lore raw
Quoted 9 lines · Al Viro
Quoted 1 line · H. Peter Anvin

The symlink support is at least 3 years old, probably more.

DEVPTS_MULTIPLE_INSTANCES wasn't anywhere near mandatory all that time... Kernel upgrade procedure that starts with "build the old kernel with that option enabled, boot it, replace /dev/ptmx with a symlink, then never try to boot earlier kernel images. Now you can safely upgrade to current kernel" is the recipe for massive self-LARTs (and resulting pitchfork crowds).

Sounds like the first thing we need to do is to remove the CONFIG_DEVPTS_MULTIPLE_INSTANCES=n code.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.