Re: tty_[un]register_devfs putting 3K structures on the stack
[EMAIL PROTECTED] said:
> If the problem only impacts User-mode Linux, it's hard for me to justify
> hanging the "critical" label on it. However I'm willing to look at the
> patch, bless it, and send it on to Linus
Below is the patch to rid tty_register_devfs and tty_unregister_devfs of the
tty_struct local.
I still think that treating it as critical deserves consideration because it's
a potentially nasty problem, and one that could be tough to debug. As Alan
pointed out, it's easier to fix the bug than it is to prove that it can't
happen on any of the other arches.
Jeff
diff -u drivers/char/tty_io.c.orig drivers/char/tty_io.c
--- drivers/char/tty_io.c.orig Tue Oct 17 00:06:04 2000
+++ drivers/char/tty_io.c Tue Oct 17 00:05:37 2000
@@ -1994,12 +1994,11 @@
{
#ifdef CONFIG_DEVFS_FS
umode_t mode = S_IFCHR | S_IRUSR | S_IWUSR;
- struct tty_struct tty;
+ kdev_t device = MKDEV (driver->major, minor);
+ int idx = minor - driver->minor_start;
char buf[32];
- tty.driver = *driver;
- tty.device = MKDEV (driver->major, minor);
- switch (tty.device) {
+ switch (device) {
case TTY_DEV:
case PTMX_DEV:
mode |= S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
@@ -2020,23 +2019,21 @@
(driver->major < UNIX98_PTY_SLAVE_MAJOR + UNIX98_NR_MAJORS) )
flags |= DEVFS_FL_CURRENT_OWNER;
# endif
- devfs_register (NULL, tty_name (, buf), flags | DEVFS_FL_DEFAULT,
+ sprintf(buf, driver->name, idx + driver->name_base);
+ devfs_register (NULL, buf, flags | DEVFS_FL_DEFAULT,
driver->major, minor, mode, _fops, NULL);
#endif /* CONFIG_DEVFS_FS */
}
-void tty_unregister_devfs (struct tty_driver *driver, unsigned minor)
+void tty_unregister_devfs (struct tty_driver *driver, unsigned int minor)
{
#ifdef CONFIG_DEVFS_FS
void * handle;
- struct tty_struct tty;
+ int idx = minor - driver->minor_start;
char buf[32];
- tty.driver = *driver;
- tty.device = MKDEV(driver->major, minor);
-
- handle = devfs_find_handle (NULL, tty_name (, buf),
- driver->major, minor,
+ sprintf(buf, driver->name, idx + driver->name_base);
+ handle = devfs_find_handle (NULL, buf, driver->major, minor,
DEVFS_SPECIAL_CHR, 0);
devfs_unregister (handle);
#endif /* CONFIG_DEVFS_FS */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
> If the problem only impacts User-mode Linux, it's hard for me to > justify > hanging the "critical" label on it. However I'm willing to look at > the > patch, bless it, and send it on to Linus (who as you know sometimes is > a > softy about such things. :-) I wasn't considering it a possible critical bug because it hurts UML. I was more considering it a potentially very nasty bug that UML happened to uncover. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
If the problem only impacts User-mode Linux, it's hard for me to justify hanging the "critical" label on it. However I'm willing to look at the patch, bless it, and send it on to Linus (who as you know sometimes is a softy about such things. :-) I wasn't considering it a possible critical bug because it hurts UML. I was more considering it a potentially very nasty bug that UML happened to uncover. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
> If the problem only impacts User-mode Linux, it's hard for me to justify > hanging the "critical" label on it. However I'm willing to look at the > patch, bless it, and send it on to Linus (who as you know sometimes is a > softy about such things. :-) > > I'm pretty sure that we'd be able to get it into 2.4.x, x > 0 at the > very least, since the patch is highly localized, won't break anything, > and is easy to test for correctness. Is it easier to fix the 3K stack use there or to prove no irq/bh path combination uses 5K ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
Date: Fri, 06 Oct 2000 16:22:11 -0500 From: Jeff Dike <[EMAIL PROTECTED]> [EMAIL PROTECTED] said: > And it's allocating a tty_struct for a really dumb reason, too. It's > just using it so it cna call tty_name. > Just replace the call to tty_name with something like this: >sprintf(buf, driver->name, idx + driver->name_base) > and make the obvious change to avoid using tty.device, and you can > avoid need to allocate a tty_struct altogether. Are you willing to consider this a critical bug that deserves to be fixed before 2.4.0? If the problem only impacts User-mode Linux, it's hard for me to justify hanging the "critical" label on it. However I'm willing to look at the patch, bless it, and send it on to Linus (who as you know sometimes is a softy about such things. :-) I'm pretty sure that we'd be able to get it into 2.4.x, x > 0 at the very least, since the patch is highly localized, won't break anything, and is easy to test for correctness. - Ted - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
Date: Fri, 06 Oct 2000 16:22:11 -0500 From: Jeff Dike [EMAIL PROTECTED] [EMAIL PROTECTED] said: And it's allocating a tty_struct for a really dumb reason, too. It's just using it so it cna call tty_name. Just replace the call to tty_name with something like this: sprintf(buf, driver-name, idx + driver-name_base) and make the obvious change to avoid using tty.device, and you can avoid need to allocate a tty_struct altogether. Are you willing to consider this a critical bug that deserves to be fixed before 2.4.0? If the problem only impacts User-mode Linux, it's hard for me to justify hanging the "critical" label on it. However I'm willing to look at the patch, bless it, and send it on to Linus (who as you know sometimes is a softy about such things. :-) I'm pretty sure that we'd be able to get it into 2.4.x, x 0 at the very least, since the patch is highly localized, won't break anything, and is easy to test for correctness. - Ted - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
If the problem only impacts User-mode Linux, it's hard for me to justify hanging the "critical" label on it. However I'm willing to look at the patch, bless it, and send it on to Linus (who as you know sometimes is a softy about such things. :-) I'm pretty sure that we'd be able to get it into 2.4.x, x 0 at the very least, since the patch is highly localized, won't break anything, and is easy to test for correctness. Is it easier to fix the 3K stack use there or to prove no irq/bh path combination uses 5K ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
[EMAIL PROTECTED] said: > And it's allocating a tty_struct for a really dumb reason, too. It's > just using it so it cna call tty_name. > Just replace the call to tty_name with something like this: > sprintf(buf, driver->name, idx + driver->name_base) > and make the obvious change to avoid using tty.device, and you can > avoid need to allocate a tty_struct altogether. Are you willing to consider this a critical bug that deserves to be fixed before 2.4.0? If so, I'm willing to make the fix and send it to whoever wants it. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
Date:Fri, 06 Oct 2000 12:01:34 -0500 From: Jeff Dike <[EMAIL PROTECTED]> tty_register_devfs and tty_unregister_devfs both declare "struct tty_struct" locals. According to gdb: (gdb) p sizeof(struct tty_struct) $20 = 3084 This eats up most of a 4K page, and on UML this is causing the stack to flow off the page for some people. Is it possible to make that tty_struct static or kmalloc it or something? And it's allocating a tty_struct for a really dumb reason, too. It's just using it so it cna call tty_name. Just replace the call to tty_name with something like this: sprintf(buf, driver->name, idx + driver->name_base) and make the obvious change to avoid using tty.device, and you can avoid need to allocate a tty_struct altogether. - Ted - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
tty_[un]register_devfs putting 3K structures on the stack
tty_register_devfs and tty_unregister_devfs both declare "struct tty_struct" locals. According to gdb: (gdb) p sizeof(struct tty_struct) $20 = 3084 This eats up most of a 4K page, and on UML this is causing the stack to flow off the page for some people. Is it possible to make that tty_struct static or kmalloc it or something? Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
Date:Fri, 06 Oct 2000 12:01:34 -0500 From: Jeff Dike [EMAIL PROTECTED] tty_register_devfs and tty_unregister_devfs both declare "struct tty_struct" locals. According to gdb: (gdb) p sizeof(struct tty_struct) $20 = 3084 This eats up most of a 4K page, and on UML this is causing the stack to flow off the page for some people. Is it possible to make that tty_struct static or kmalloc it or something? And it's allocating a tty_struct for a really dumb reason, too. It's just using it so it cna call tty_name. Just replace the call to tty_name with something like this: sprintf(buf, driver-name, idx + driver-name_base) and make the obvious change to avoid using tty.device, and you can avoid need to allocate a tty_struct altogether. - Ted - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: tty_[un]register_devfs putting 3K structures on the stack
[EMAIL PROTECTED] said: And it's allocating a tty_struct for a really dumb reason, too. It's just using it so it cna call tty_name. Just replace the call to tty_name with something like this: sprintf(buf, driver-name, idx + driver-name_base) and make the obvious change to avoid using tty.device, and you can avoid need to allocate a tty_struct altogether. Are you willing to consider this a critical bug that deserves to be fixed before 2.4.0? If so, I'm willing to make the fix and send it to whoever wants it. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
