Linux Container Development
 help / color / mirror / Atom feed
From: sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
To: hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org,
	alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org
Cc: kyle-hoO6YkzgTuCM0SS3m2neIg@public.gmane.org,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
	bastian-yyjItF7Rl6lg9hUCZPvPmw@public.gmane.org,
	containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org,
	xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
Subject: [RFC][PATCH 2/3] Move some init_dev() code to callers
Date: Mon, 25 Aug 2008 13:23:14 -0700	[thread overview]
Message-ID: <20080825202314.GA318@us.ibm.com> (raw)
In-Reply-To: <20080825201110.GA32440-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>


From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [RFC][PATCH 2/3] Move some init_dev() code to callers

init_dev() tries to find a tty and if it finds an existing tty, does
a 'fast' open. If its not an existing tty, init_dev does a slower
first time open requiring allocation and complex initialization.

All these seem to make the code more complex. When opening /dev/tty,
the caller already has the tty so there is no need to find it. Further
the fast and slow opens in init_dev() don't really share much code
and could be in separate functions.

With only two callers, init_dev() does not really need to be that
generalized and some of the pieces can be moved into the callers.

---
 drivers/char/tty_io.c |   71 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 47 insertions(+), 24 deletions(-)

Index: linux-next/drivers/char/tty_io.c
===================================================================
--- linux-next.orig/drivers/char/tty_io.c	2008-08-25 12:31:15.000000000 -0700
+++ linux-next/drivers/char/tty_io.c	2008-08-25 12:54:35.000000000 -0700
@@ -1322,20 +1322,6 @@ static int init_dev(struct tty_driver *d
 	struct ktermios *ltp, **ltp_loc, *o_ltp, **o_ltp_loc;
 	int retval = 0;
 
-	tty = find_tty(driver, idx);
-	if (IS_ERR(tty)) {
-		retval = PTR_ERR(tty);
-		goto end_init;
-	}
-
-	if (tty) {
-		retval = fast_tty_open(tty);
-		if (retval)
-			return retval;
-		*ret_tty = tty;
-		return 0;
-	}
-
 	/* Check if pty master is being opened multiple times */
 	if (driver->subtype == PTY_TYPE_MASTER &&
 		(driver->flags & TTY_DRIVER_DEVPTS_MEM) && !first_ok) {
@@ -1476,9 +1462,7 @@ static int init_dev(struct tty_driver *d
 
 	if (retval)
 		goto release_mem_out;
-	goto success;
 
-success:
 	*ret_tty = tty;
 
 	/* All paths come through here to release the mutex */
@@ -1853,14 +1837,15 @@ static void release_dev(struct file *fil
  *	The termios state of a pty is reset on first open so that
  *	settings don't persist across reuse.
  *
- *	Locking: tty_mutex protects tty, get_tty_driver and init_dev work.
+ *	Locking: tty_mutex protects tty, get_tty_driver, find_tty,
+ *		fast_tty_open and init_dev work.
  *		 tty->count should protect the rest.
  *		 ->siglock protects ->signal/->sighand
  */
 
 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;
@@ -1917,8 +1902,19 @@ retry_open:
 		return -ENODEV;
 	}
 got_driver:
-	retval = init_dev(driver, index, &tty, 0);
+	if (!tty) {
+		tty = find_tty(driver, index);
+		if (IS_ERR(tty))
+			return PTR_ERR(tty);
+	}
+
+	if (tty)
+		retval = fast_tty_open(tty);
+	else
+		retval = init_dev(driver, index, &tty, 0);
+
 	mutex_unlock(&tty_mutex);
+
 	if (retval)
 		return retval;
 
@@ -1986,8 +1982,24 @@ static int tty_open(struct inode *inode,
 }
 
 
-
 #ifdef CONFIG_UNIX98_PTYS
+
+static struct tty_struct *find_open_tty(struct tty_driver *driver, int index)
+{
+	int retval;
+	struct tty_struct *tty;
+
+	tty = find_tty(driver, index);
+	if (!tty || IS_ERR(tty))
+		return tty;
+
+	retval = fast_tty_open(tty);
+	if (retval)
+		tty = ERR_PTR(retval);
+
+	return tty;
+}
+
 /**
  *	ptmx_open		-	open a unix 98 pty master
  *	@inode: inode of device file
@@ -1995,15 +2007,15 @@ static int tty_open(struct inode *inode,
  *
  *	Allocate a unix98 pty master device from the ptmx driver.
  *
- *	Locking: tty_mutex protects theinit_dev work. tty->count should
- * 		protect the rest.
+ *	Locking: tty_mutex protects the find_open_tty and init_dev work.
+ *		tty->count should protect the rest.
  *		allocated_ptys_lock handles the list of free pty numbers
  */
 
 static int __ptmx_open(struct inode *inode, struct file *filp)
 {
 	struct tty_struct *tty;
-	int retval;
+	int retval = 0;
 	int index;
 
 	nonseekable_open(inode, filp);
@@ -2014,7 +2026,18 @@ static int __ptmx_open(struct inode *ino
 		return index;
 
 	mutex_lock(&tty_mutex);
-	retval = init_dev(ptm_driver, index, &tty, 1);
+
+	/*
+	 * TODO: We just allocated the index, will find_open_tty() ever
+	 *	 find a tty ? Keep the find for now for compatiblity with
+	 *	 old init_dev().
+	 */
+	tty = find_open_tty(ptm_driver, index);
+	if (IS_ERR(tty))
+		retval = PTR_ERR(tty);
+	else if (!tty)
+		retval = init_dev(ptm_driver, index, &tty, 1);
+
 	mutex_unlock(&tty_mutex);
 
 	if (retval)

  parent reply	other threads:[~2008-08-25 20:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-25 20:11 [RFC][PATCH 1/3] Move parts of init_dev() into new functions sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found] ` <20080825201110.GA32440-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-25 20:11   ` Alan Cox
     [not found]     ` <20080825211146.70b4af63-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2008-08-25 22:01       ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]         ` <20080825220125.GA1084-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-25 22:21           ` H. Peter Anvin
     [not found]             ` <48B33072.4080509-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2008-08-25 22:20               ` Alan Cox
     [not found]                 ` <20080825232003.3574a181-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2008-08-26  0:02                   ` H. Peter Anvin
     [not found]                     ` <48B34818.2000400-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2008-08-26  9:44                       ` Alan Cox
     [not found]                         ` <20080826104445.06d36dd2-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2008-08-26 16:40                           ` H. Peter Anvin
     [not found]                             ` <48B431F4.90201-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2008-08-26 16:49                               ` Alan Cox
     [not found]                                 ` <20080826174921.6bfbf989-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2008-08-26 17:40                                   ` H. Peter Anvin
2008-08-25 23:57               ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]                 ` <20080825235736.GA2959-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-26  0:04                   ` H. Peter Anvin
     [not found]                     ` <48B348A7.5020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2008-08-26  1:18                       ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]                         ` <20080826011807.GB2959-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-26  2:13                           ` H. Peter Anvin
2008-08-25 20:23   ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA [this message]
     [not found]     ` <20080825202314.GA318-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-28 20:17       ` [RFC][PATCH 2/3] Move some init_dev() code to callers sukadev-r/Jw6+rmf7HQT0dZR+AlfA
2008-08-25 20:23   ` [RFC][PATCH 3/3] Rename and change init_dev() prototype sukadev-r/Jw6+rmf7HQT0dZR+AlfA
2008-08-26 11:09   ` [RFC][PATCH 1/3] Move parts of init_dev() into new functions Alan Cox
2008-08-28 20:25   ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080828202520.GG24075-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-08-28 21:00       ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080825202314.GA318@us.ibm.com \
    --to=sukadev-r/jw6+rmf7hqt0dzr+alfa@public.gmane.org \
    --cc=alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org \
    --cc=bastian-yyjItF7Rl6lg9hUCZPvPmw@public.gmane.org \
    --cc=containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
    --cc=kyle-hoO6YkzgTuCM0SS3m2neIg@public.gmane.org \
    --cc=xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox