All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.