mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* Re: [PATCH 60/80] tty: extract the pty init time special cases
@ 2009-01-22  2:16 bmckinlay
  2009-01-22 12:40 ` Alan Cox
  0 siblings, 1 reply; 8+ messages in thread
From: bmckinlay @ 2009-01-22  2:16 UTC (permalink / raw)
  To: linux-kernel

My question/comment is in regards to the following patch indicated below 
that was posted on October 13, 2008.
 
>  From Alan Cox <>
> Subject       [PATCH 60/80] tty: extract the pty init time special cases
> Date  Mon, 13 Oct 2008 10:42:39 +0100
>
> From: Alan Cox <alan@redhat.com>
> 
> The majority of the remaining init_dev code is pty special cases. We 
> refactor this code into the driver->install method.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> ---
>  drivers/char/pty.c    |  126 ++++++++++++++++++++++++++++---
>  drivers/char/tty_io.c |  198 
+++++++++++++++++--------------------------------
>  include/linux/tty.h   |    5 +
>  3 files changed, 187 insertions(+), 142 deletions(-)
> 

This patch seems to break any other pseudo tty drivers, (ie driver->type 
==TTY_DRIVER_TYPE_PTY) since not all the tty functions needed for the 
driver's new driver->install method are exported from tty_io.c.  The 
function symbols that need to be exported from /drivers/char/tty_io.c are 
the following:

   EXPORT_SYMBOL_GPL(alloc_tty_struct);
   EXPORT_SYMBOL_GPL(free_tty_struct);
   EXPORT_SYMBOL_GPL(tty_init_termios);
   EXPORT_SYMBOL_GPL(initialize_tty_struct);

I've added these lines and recompiled a 2.6.28 kernel along with adding 
new driver->install methods to my master and slave pseudo tty driver and 
it seems to work fine now.

Before I go ahead and post a patch, does anyone see any problems with 
this?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-22  2:16 [PATCH 60/80] tty: extract the pty init time special cases bmckinlay
@ 2009-01-22 12:40 ` Alan Cox
  2009-01-28 20:36   ` bmckinlay
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2009-01-22 12:40 UTC (permalink / raw)
  To: bmckinlay; +Cc: linux-kernel

> This patch seems to break any other pseudo tty drivers, (ie driver->type 
> ==TTY_DRIVER_TYPE_PTY) since not all the tty functions needed for the 
> driver's new driver->install method are exported from tty_io.c.  The 
> function symbols that need to be exported from /drivers/char/tty_io.c are 

I did ask at the time what other PTY drivers there were as and
what .config case in the kernel needed those symbols. If you only need
them for an out of tree driver then they can stay out of tree until that
driver is merged.

Alan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-22 12:40 ` Alan Cox
@ 2009-01-28 20:36   ` bmckinlay
  2009-01-28 20:47     ` Alan Cox
  0 siblings, 1 reply; 8+ messages in thread
From: bmckinlay @ 2009-01-28 20:36 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote on 01/22/2009 07:40:42 AM:
> > This patch seems to break any other pseudo tty drivers, (ie 
driver->type
> > ==TTY_DRIVER_TYPE_PTY) since not all the tty functions needed for the
> > driver's new driver->install method are exported from tty_io.c.  The
> > function symbols that need to be exported from /drivers/char/tty_io.c 
are

> I did ask at the time what other PTY drivers there were as and
> what .config case in the kernel needed those symbols. If you only need
> them for an out of tree driver then they can stay out of tree until that
> driver is merged.

So you are saying that if I merge my ptyx module into the kernel tree THEN 
these
exports would be added?  They would have to be, since I would still want 
my 
driver to be a module instead of statically linked with the kernel like 
pty is.

I think the fact that you had to "ask at the time what other PTY drivers 
there 
were" indicates that these symbols should have been simply exported to 
avoid 
any problems with out of tree PTY drivers.

By not exporting these symbols it means that the Linux kernel is now 
forcing 
anyone who wants to develop a PTY type TTY driver to be merged into the 
kernel 
tree by not exporting these TTY functions. This doesn't seem very "open 
and 
flexible".






^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-28 20:36   ` bmckinlay
@ 2009-01-28 20:47     ` Alan Cox
  2009-01-29 18:30       ` bmckinlay
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2009-01-28 20:47 UTC (permalink / raw)
  To: bmckinlay; +Cc: linux-kernel

> So you are saying that if I merge my ptyx module into the kernel tree THEN 
> these
> exports would be added?  They would have to be, since I would still want 

It would then seem reasonable to do so

> my 
> driver to be a module instead of statically linked with the kernel like 
> pty is.

Sure - although it might make more sense to add any neede features to the
generic pty driver - hard to tell when you don't seem interested in
discussing that.

> I think the fact that you had to "ask at the time what other PTY drivers 
> there 
> were" indicates that these symbols should have been simply exported to 
> avoid 
> any problems with out of tree PTY drivers.

The linux kernel doesn't carry extra symbols for out of tree drivers. Why
should it - even keeping an index of them would be hard and it would be
impossible to know which ones are in use. Plus they change regularly -
the tty layer will change further soon for example, it's just having a
settling in period before the next big changes.

> By not exporting these symbols it means that the Linux kernel is now 
> forcing 
> anyone who wants to develop a PTY type TTY driver to be merged into the 
> kernel 
> tree by not exporting these TTY functions. This doesn't seem very "open 
> and 
> flexible".

No you can provide the GPL symbol exports with the same patch as your pty
driver. There are millions of Linux users and each unused EXPORT_SYMBOL
costs memory to all those people - why should they carry that burden ?

Alan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-28 20:47     ` Alan Cox
@ 2009-01-29 18:30       ` bmckinlay
  2009-01-29 19:34         ` Alan Cox
  0 siblings, 1 reply; 8+ messages in thread
From: bmckinlay @ 2009-01-29 18:30 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel

> > my driver to be a module instead of statically linked with the kernel 
like
> > pty is.

> Sure - although it might make more sense to add any neede features to 
the
> generic pty driver - hard to tell when you don't seem interested in
> discussing that.

It's not necessarily that I'm not interested but more that I'm a little
reluctant to do it for the following reasons:

The unknown:
  - have never personally submitted anything to the kernel, need to find 
out 
    what the requirements are, how much documentation you need etc
  - no guarantee how long this will take and that my submission will even 
be 
    accepted.  The longer it takes the more kernel releases that will be
    released where my driver no longer works
Loss of control:
  - my ptyx driver is just one part of the product, it has a daemon part 
and
    of coarse configurators. If the driver is put in the kernel then we 
lose
    the guarantee that a customer will be using compatible versions of 
these 
    3 parts of the product.  Now they are all packaged together
Work involved: 
  - my current ptyx driver is conditionally compiled up the whazoo since 
it 
    works on 2.2.x, 2.4.x and 2.6.x kernels.  Will have to pull out the 
2.6.x
    specific code in order to submit it.
  - will have to add the driver to the kernel config as a configurable 
module
  - any required documentation explaining how the driver works and 
describing
    the API for the master side of the PTY driver ?
  - actually creating the patch and submitting it.

> > By not exporting these symbols it means that the Linux kernel is now
> > forcing anyone who wants to develop a PTY type TTY driver to be merged 

> > into the kernel tree by not exporting these TTY functions. This 
doesn't 
> > seem very "open and flexible".

> No you can provide the GPL symbol exports with the same patch as your 
pty
> driver. There are millions of Linux users and each unused EXPORT_SYMBOL
> costs memory to all those people - why should they carry that burden ?

Given the fact that it in this day and age it is unaceptable to require 
customers to recompile their kernel in order to use a new driver I beleive
my point was lost here.  I was referring to "out of kernel tree" driver
development.
I'll explain my point another way.
As mentioned above the Linux kernel has supported "out of tree" PTY type 
TTY 
drivers all the way back to kernel 2.2.x and up to 2.6.27. To do this the 
kernel exports symbols like 
  - alloc_tty_driver(), tty_set_operations() or tty_register_driver() used 

    in TTY driver's module init functions
  - tty_hangup() used in driver close operations/methods
  - tty_insert_flip_char() or tty_flip_buffer_push() used in ioctl 
    operations/methods.
  - tty_unregister_driver() used in TTY driver module exit functions
 
Now in this patch it was decided to push some of the PTY specific logic 
from 
tty_init_dev() down to the PTY driver by introducing a new install method 
for
PTY drivers.  This new install method for the PTY driver needs to allocate 
the
slave's TTY and initialize it but the TTY functions required to do this 
were not
exported.
All I'm asking is for the Linux kernel to maintain it's support for "out 
of tree" 
PTY type drivers.







^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-29 18:30       ` bmckinlay
@ 2009-01-29 19:34         ` Alan Cox
  2009-02-02 15:42           ` bmckinlay
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2009-01-29 19:34 UTC (permalink / raw)
  To: bmckinlay; +Cc: linux-kernel

>   - any required documentation explaining how the driver works and 
> describing
>     the API for the master side of the PTY driver ?

The first most useful part would be understand what your pty driver does
and is required that is not in the base one. Just an idea of what extra
APIs it provides would be quite useful. If it is stuff like virtual modem
lines then that is stuff I want to see supported in the base kernel
anyway.

> As mentioned above the Linux kernel has supported "out of tree" PTY type 
> TTY 
> drivers all the way back to kernel 2.2.x and up to 2.6.27. To do this the 

By chance.. as to policies about kernel recompiles, that is between you
and the vendors. Maybe the vendors will add the symbols to their tree for
you ?

Equally look at the positives - if the facilities you need are in the
base kernel your customers won't run into arguments between yourselves
and the base kernel vendor over support.

Alan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 60/80] tty: extract the pty init time special cases
  2009-01-29 19:34         ` Alan Cox
@ 2009-02-02 15:42           ` bmckinlay
  0 siblings, 0 replies; 8+ messages in thread
From: bmckinlay @ 2009-02-02 15:42 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel

> The first most useful part would be understand what your pty driver does
> and is required that is not in the base one. Just an idea of what extra
> APIs it provides would be quite useful. If it is stuff like virtual 
modem
> lines then that is stuff I want to see supported in the base kernel
> anyway.

Our pty driver basically creates a virtual serial TTY on the slave side 
which 
will allow the application to control a remote device server serial port 
across a LAN network.  The application can control things like baud rate,
parity, stop bits, HW & SW flow control, and monitor and control modem 
signals. 

Below are some defines that would explain how we extended the base pty.
One note is the our diver is based on the legacy/bsd pty not the unix98
pty, so if it was decided that this extension was to be merged into the
base pty then the code would have to be ported to the unix98 sections as
well.

/* 
   There aren't enough bits available in tty->ctrl_status for the events 
that
   we need to signal to the daemon, so we will use 1 bit to indicate that
   the daemon needs to make an IOCTL (PTX_IOCXCSGET) to get the real 
status.
*/
#define TIOCPKT_XCTRLSTATUS     0x40

//  bit definition for ptyx_struct.ctrl_status
#define XCTRLSTATUS_SIGNALS     0x00000001      // update DTR, RTS signals
#define XCTRLSTATUS_TERMIOS     0x00000002      // update termios settings
#define XCTRLSTATUS_BREAK       0x00000004      // send break signal
#define XCTRLSTATUS_OPEN        0x00000008      // slave port opened
#define XCTRLSTATUS_CLOSE       0x00000010      // slave port closed
#define XCTRLSTATUS_BREAKON     0x00000020      // send break on
#define XCTRLSTATUS_BREAKOFF    0x00000040      // send break off
#define XCTRLSTATUS_DRAIN       0x00000080      // drain port

// IOCTLs handled by master ptyx driver
#define PTX_IOC         ('p' << 24) + ('t' << 16) + ('x' << 8)
#define PTX_IOCMSET     PTX_IOC + 1     // set modem signal
#define PTX_IOCLSET     PTX_IOC + 2     // set line status info
#define PTX_IOCXCSGET   PTX_IOC + 3     // get extended control status
#define PTX_IOCDRAINEND PTX_IOC + 4     // drain complete
#define PTX_IOCNETSTAT  PTX_IOC + 5     // update network
#define PTX_IOCOWTSET   PTX_IOC + 6     // give ptyx, the open wait time 
in msec
                                                // -1=ignore network 
status, -2=wait forever



 


  Barry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 60/80] tty: extract the pty init time special cases
  2008-10-13  9:31 [PATCH 00/80] TTY updates for 2.6.28 Alan Cox
@ 2008-10-13  9:42 ` Alan Cox
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Cox @ 2008-10-13  9:42 UTC (permalink / raw)
  To: torvalds, linux-kernel

From: Alan Cox <alan@redhat.com>

The majority of the remaining init_dev code is pty special cases. We
refactor this code into the driver->install method.

Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/char/pty.c    |  126 ++++++++++++++++++++++++++++---
 drivers/char/tty_io.c |  198 +++++++++++++++++--------------------------------
 include/linux/tty.h   |    5 +
 3 files changed, 187 insertions(+), 142 deletions(-)


diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index c984500..c5a192d 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -227,7 +227,58 @@ static void pty_set_termios(struct tty_struct *tty, struct ktermios *old_termios
         tty->termios->c_cflag |= (CS8 | CREAD);
 }
 
+static int pty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	struct tty_struct *o_tty;
+	int idx = tty->index;
+	int retval;
+
+	o_tty = alloc_tty_struct();
+	if (!o_tty)
+		return -ENOMEM;
+	if (!try_module_get(driver->other->owner)) {
+		/* This cannot in fact currently happen */
+		free_tty_struct(o_tty);
+		return -ENOMEM;
+	}
+	initialize_tty_struct(o_tty, driver->other, idx);
+
+	/* We always use new tty termios data so we can do this
+	   the easy way .. */
+	retval = tty_init_termios(tty);
+	if (retval)
+		goto free_mem_out;
+
+	retval = tty_init_termios(o_tty);
+	if (retval) {
+		tty_free_termios(tty);
+		goto free_mem_out;
+	}
+	
+	/*
+	 * Everything allocated ... set up the o_tty structure.
+	 */
+	driver->other->ttys[idx] = o_tty;
+	tty_driver_kref_get(driver->other);
+	if (driver->subtype == PTY_TYPE_MASTER)
+		o_tty->count++;
+	/* Establish the links in both directions */
+	tty->link   = o_tty;
+	o_tty->link = tty;
+
+	tty_driver_kref_get(driver);
+	tty->count++;
+	driver->ttys[idx] = tty;
+	return 0;
+free_mem_out:
+	module_put(o_tty->driver->owner);
+	free_tty_struct(o_tty);
+	return -ENOMEM;
+}
+
+
 static const struct tty_operations pty_ops = {
+	.install = pty_install,
 	.open = pty_open,
 	.close = pty_close,
 	.write = pty_write,
@@ -332,6 +383,7 @@ static inline void legacy_pty_init(void) { }
 int pty_limit = NR_UNIX98_PTY_DEFAULT;
 static int pty_limit_min = 0;
 static int pty_limit_max = NR_UNIX98_PTY_MAX;
+static int pty_count = 0;
 
 static struct cdev ptmx_cdev;
 
@@ -351,6 +403,7 @@ static struct ctl_table pty_table[] = {
 		.procname	= "nr",
 		.maxlen		= sizeof(int),
 		.mode		= 0444,
+		.data		= &pty_count,
 		.proc_handler	= &proc_dointvec,
 	}, {
 		.ctl_name	= 0
@@ -426,7 +479,7 @@ static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver, int idx)
 	return tty;
 }
 
-static void pty_shutdown(struct tty_struct *tty)
+static void pty_unix98_shutdown(struct tty_struct *tty)
 {
 	/* We have our own method as we don't use the tty index */
 	kfree(tty->termios);
@@ -436,19 +489,71 @@ static void pty_shutdown(struct tty_struct *tty)
 /* We have no need to install and remove our tty objects as devpts does all
    the work for us */
 
-static int pty_install(struct tty_driver *driver, struct tty_struct *tty)
+static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
 {
+	struct tty_struct *o_tty;
+	int idx = tty->index;
+
+	o_tty = alloc_tty_struct();
+	if (!o_tty)
+		return -ENOMEM;
+	if (!try_module_get(driver->other->owner)) {
+		/* This cannot in fact currently happen */
+		free_tty_struct(o_tty);
+		return -ENOMEM;
+	}
+	initialize_tty_struct(o_tty, driver->other, idx);
+
+	tty->termios = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
+	if (tty->termios == NULL)
+		goto free_mem_out;
+	*tty->termios = driver->init_termios;
+	tty->termios_locked = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
+	if (tty->termios_locked == NULL)
+		goto free_mem_out;
+	o_tty->termios = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
+	if (o_tty->termios == NULL)
+		goto free_mem_out;
+	*o_tty->termios = driver->other->init_termios;
+	o_tty->termios_locked = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
+	if (o_tty->termios_locked == NULL)
+		goto free_mem_out;
+
+	tty_driver_kref_get(driver->other);
+	if (driver->subtype == PTY_TYPE_MASTER)
+		o_tty->count++;
+	/* Establish the links in both directions */
+	tty->link   = o_tty;
+	o_tty->link = tty;
+	/*
+	 * All structures have been allocated, so now we install them.
+	 * Failures after this point use release_tty to clean up, so
+	 * there's no need to null out the local pointers.
+	 */
+	tty_driver_kref_get(driver);
+	tty->count++;
+	pty_count++;
 	return 0;
+free_mem_out:
+	kfree(o_tty->termios);
+	module_put(o_tty->driver->owner);
+	free_tty_struct(o_tty);
+	kfree(tty->termios_locked);
+	kfree(tty->termios);
+	free_tty_struct(tty);
+	module_put(driver->owner);
+	return -ENOMEM;
 }
 
-static void pty_remove(struct tty_driver *driver, struct tty_struct *tty)
+static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty)
 {
+	pty_count--;
 }
 
 static const struct tty_operations ptm_unix98_ops = {
 	.lookup = ptm_unix98_lookup,
-	.install = pty_install,
-	.remove = pty_remove,
+	.install = pty_unix98_install,
+	.remove = pty_unix98_remove,
 	.open = pty_open,
 	.close = pty_close,
 	.write = pty_write,
@@ -458,13 +563,13 @@ static const struct tty_operations ptm_unix98_ops = {
 	.unthrottle = pty_unthrottle,
 	.set_termios = pty_set_termios,
 	.ioctl = pty_unix98_ioctl,
-	.shutdown = pty_shutdown
+	.shutdown = pty_unix98_shutdown
 };
 
 static const struct tty_operations pty_unix98_ops = {
 	.lookup = pts_unix98_lookup,
-	.install = pty_install,
-	.remove = pty_remove,
+	.install = pty_unix98_install,
+	.remove = pty_unix98_remove,
 	.open = pty_open,
 	.close = pty_close,
 	.write = pty_write,
@@ -473,6 +578,7 @@ static const struct tty_operations pty_unix98_ops = {
 	.chars_in_buffer = pty_chars_in_buffer,
 	.unthrottle = pty_unthrottle,
 	.set_termios = pty_set_termios,
+	.shutdown = pty_unix98_shutdown
 };
 
 /**
@@ -589,10 +695,6 @@ static void __init unix98_pty_init(void)
 	if (tty_register_driver(pts_driver))
 		panic("Couldn't register Unix98 pts driver");
 
-	/* FIXME: WTF */
-#if 0	
-	pty_table[1].data = &ptm_driver->refcount;
-#endif	
 	register_sysctl_table(pty_root_table);	
 
 	/* Now create the /dev/ptmx special device */
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index b0ad488..e881e9e 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -136,8 +136,6 @@ LIST_HEAD(tty_drivers);			/* linked list of tty drivers */
 DEFINE_MUTEX(tty_mutex);
 EXPORT_SYMBOL(tty_mutex);
 
-static void initialize_tty_struct(struct tty_struct *tty);
-
 static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
 static ssize_t tty_write(struct file *, const char __user *, size_t, loff_t *);
 ssize_t redirected_tty_write(struct file *, const char __user *,
@@ -166,7 +164,7 @@ static void proc_set_tty(struct task_struct *tsk, struct tty_struct *tty);
  *	Locking: none
  */
 
-static struct tty_struct *alloc_tty_struct(void)
+struct tty_struct *alloc_tty_struct(void)
 {
 	return kzalloc(sizeof(struct tty_struct), GFP_KERNEL);
 }
@@ -180,7 +178,7 @@ static struct tty_struct *alloc_tty_struct(void)
  *	Locking: none. Must be called after tty is definitely unused
  */
 
-static inline void free_tty_struct(struct tty_struct *tty)
+void free_tty_struct(struct tty_struct *tty)
 {
 	kfree(tty->write_buf);
 	tty_buffer_free_all(tty);
@@ -1227,22 +1225,70 @@ struct tty_struct *tty_driver_lookup_tty(struct tty_driver *driver, int idx)
 }
 
 /**
+ *	tty_init_termios	-  helper for termios setup
+ *	@tty: the tty to set up
+ *
+ *	Initialise the termios structures for this tty. Thus runs under
+ *	the tty_mutex currently so we can be relaxed about ordering.
+ */
+
+int tty_init_termios(struct tty_struct *tty)
+{
+	struct ktermios *tp, *ltp;
+	int idx = tty->index;
+
+	tp = tty->driver->termios[idx];
+	ltp = tty->driver->termios_locked[idx];
+	if (tp == NULL) {
+		WARN_ON(ltp != NULL);
+		tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
+		ltp = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
+		if (tp == NULL || ltp == NULL) {
+			kfree(tp);
+			kfree(ltp);
+			return -ENOMEM;
+		}
+		memcpy(tp, &tty->driver->init_termios,
+						sizeof(struct ktermios));
+		tty->driver->termios[idx] = tp;
+		tty->driver->termios_locked[idx] = ltp;
+	}
+	tty->termios = tp;
+	tty->termios_locked = ltp;
+
+	/* Compatibility until drivers always set this */
+	tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios);
+	tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios);
+	return 0;
+}
+
+/**
  *	tty_driver_install_tty() - install a tty entry in the driver
  *	@driver: the driver for the tty
  *	@tty: the tty
  *
  *	Install a tty object into the driver tables. The tty->index field
- *	will be set by the time this is called.
+ *	will be set by the time this is called. This method is responsible
+ *	for ensuring any need additional structures are allocated and
+ *	configured.
  *
  *	Locking: tty_mutex for now
  */
 static int tty_driver_install_tty(struct tty_driver *driver,
 						struct tty_struct *tty)
 {
+	int idx = tty->index;
+
 	if (driver->ops->install)
 		return driver->ops->install(driver, tty);
-	driver->ttys[tty->index] = tty;
-	return 0;
+
+	if (tty_init_termios(tty) == 0) {
+		tty_driver_kref_get(driver);
+		tty->count++;
+		driver->ttys[idx] = tty;
+		return 0;
+	}
+	return -ENOMEM;
 }
 
 /**
@@ -1327,9 +1373,7 @@ static int tty_reopen(struct tty_struct *tty)
 struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
 								int first_ok)
 {
-	struct tty_struct *tty, *o_tty;
-	struct ktermios *tp, **tp_loc, *o_tp, **o_tp_loc;
-	struct ktermios *ltp, **ltp_loc, *o_ltp, **o_ltp_loc;
+	struct tty_struct *tty;
 	int retval;
 
 	/* check whether we're reopening an existing tty */
@@ -1361,118 +1405,17 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
 	if (!try_module_get(driver->owner))
 		return ERR_PTR(-ENODEV);
 
-	o_tty = NULL;
-	tp = o_tp = NULL;
-	ltp = o_ltp = NULL;
-
 	tty = alloc_tty_struct();
 	if (!tty)
 		goto fail_no_mem;
-	initialize_tty_struct(tty);
-	tty->driver = driver;
-	tty->ops = driver->ops;
-	tty->index = idx;
-	tty_line_name(driver, idx, tty->name);
-
-	if (driver->flags & TTY_DRIVER_DEVPTS_MEM) {
-		tp_loc = &tty->termios;
-		ltp_loc = &tty->termios_locked;
-	} else {
-		tp_loc = &driver->termios[idx];
-		ltp_loc = &driver->termios_locked[idx];
-	}
-
-	if (!*tp_loc) {
-		tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
-		if (!tp)
-			goto free_mem_out;
-		*tp = driver->init_termios;
-	}
-
-	if (!*ltp_loc) {
-		ltp = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
-		if (!ltp)
-			goto free_mem_out;
-	}
-
-	if (driver->type == TTY_DRIVER_TYPE_PTY) {
-		o_tty = alloc_tty_struct();
-		if (!o_tty)
-			goto free_mem_out;
-		if (!try_module_get(driver->other->owner)) {
-			/* This cannot in fact currently happen */
-			free_tty_struct(o_tty);
-			o_tty = NULL;
-			goto free_mem_out;
-		}
-		initialize_tty_struct(o_tty);
-		o_tty->driver = driver->other;
-		o_tty->ops = driver->ops;
-		o_tty->index = idx;
-		tty_line_name(driver->other, idx, o_tty->name);
-
-		if (driver->flags & TTY_DRIVER_DEVPTS_MEM) {
-			o_tp_loc = &o_tty->termios;
-			o_ltp_loc = &o_tty->termios_locked;
-		} else {
-			o_tp_loc = &driver->other->termios[idx];
-			o_ltp_loc = &driver->other->termios_locked[idx];
-		}
-
-		if (!*o_tp_loc) {
-			o_tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
-			if (!o_tp)
-				goto free_mem_out;
-			*o_tp = driver->other->init_termios;
-		}
-
-		if (!*o_ltp_loc) {
-			o_ltp = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
-			if (!o_ltp)
-				goto free_mem_out;
-		}
-
-		/*
-		 * Everything allocated ... set up the o_tty structure.
-		 */
-		if (!(driver->other->flags & TTY_DRIVER_DEVPTS_MEM))
-			driver->other->ttys[idx] = o_tty;
-		if (!*o_tp_loc)
-			*o_tp_loc = o_tp;
-		if (!*o_ltp_loc)
-			*o_ltp_loc = o_ltp;
-		o_tty->termios = *o_tp_loc;
-		o_tty->termios_locked = *o_ltp_loc;
-		tty_driver_kref_get(driver->other);
-		if (driver->subtype == PTY_TYPE_MASTER)
-			o_tty->count++;
-
-		/* Establish the links in both directions */
-		tty->link   = o_tty;
-		o_tty->link = tty;
-	}
-
-	/*
-	 * All structures have been allocated, so now we install them.
-	 * Failures after this point use release_tty to clean up, so
-	 * there's no need to null out the local pointers.
-	 */
-
-	if (!*tp_loc)
-		*tp_loc = tp;
-	if (!*ltp_loc)
-		*ltp_loc = ltp;
-	tty->termios = *tp_loc;
-	tty->termios_locked = *ltp_loc;
-	/* Compatibility until drivers always set this */
-	tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios);
-	tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios);
-	tty_driver_kref_get(driver);
-	tty->count++;
+	initialize_tty_struct(tty, driver, idx);
 
 	retval = tty_driver_install_tty(driver, tty);
-	if (retval < 0)
-		goto release_mem_out;
+	if (retval < 0) {
+		free_tty_struct(tty);
+		module_put(driver->owner);
+		return ERR_PTR(retval);
+	}
 
 	/*
 	 * Structures all installed ... call the ldisc open routines.
@@ -1480,22 +1423,11 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
 	 * to decrement the use counts, as release_tty doesn't care.
 	 */
 
-	retval = tty_ldisc_setup(tty, o_tty);
+	retval = tty_ldisc_setup(tty, tty->link);
 	if (retval)
 		goto release_mem_out;
 	return tty;
 
-	/* Release locally allocated memory ... nothing placed in slots */
-free_mem_out:
-	kfree(o_tp);
-	if (o_tty) {
-		module_put(o_tty->driver->owner);
-		free_tty_struct(o_tty);
-	}
-	kfree(ltp);
-	kfree(tp);
-	free_tty_struct(tty);
-
 fail_no_mem:
 	module_put(driver->owner);
 	return ERR_PTR(-ENOMEM);
@@ -2852,7 +2784,8 @@ EXPORT_SYMBOL(do_SAK);
  *	Locking: none - tty in question must not be exposed at this point
  */
 
-static void initialize_tty_struct(struct tty_struct *tty)
+void initialize_tty_struct(struct tty_struct *tty,
+		struct tty_driver *driver, int idx)
 {
 	memset(tty, 0, sizeof(struct tty_struct));
 	kref_init(&tty->kref);
@@ -2873,6 +2806,11 @@ static void initialize_tty_struct(struct tty_struct *tty)
 	spin_lock_init(&tty->ctrl_lock);
 	INIT_LIST_HEAD(&tty->tty_files);
 	INIT_WORK(&tty->SAK_work, do_SAK_work);
+
+	tty->driver = driver;
+	tty->ops = driver->ops;
+	tty->index = idx;
+	tty_line_name(driver, idx, tty->name);
 }
 
 /**
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 54523a3..3c7c757 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -401,9 +401,14 @@ extern dev_t tty_devnum(struct tty_struct *tty);
 extern void proc_clear_tty(struct task_struct *p);
 extern struct tty_struct *get_current_tty(void);
 extern void tty_default_fops(struct file_operations *fops);
+extern struct tty_struct *alloc_tty_struct(void);
+extern void free_tty_struct(struct tty_struct *tty);
+extern void initialize_tty_struct(struct tty_struct *tty,
+		struct tty_driver *driver, int idx);
 extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
 								int first_ok);
 extern void tty_release_dev(struct file *filp);
+extern int tty_init_termios(struct tty_struct *tty);
 
 extern struct mutex tty_mutex;
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-02-02 15:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-22  2:16 [PATCH 60/80] tty: extract the pty init time special cases bmckinlay
2009-01-22 12:40 ` Alan Cox
2009-01-28 20:36   ` bmckinlay
2009-01-28 20:47     ` Alan Cox
2009-01-29 18:30       ` bmckinlay
2009-01-29 19:34         ` Alan Cox
2009-02-02 15:42           ` bmckinlay
  -- strict thread matches above, loose matches on Subject: below --
2008-10-13  9:31 [PATCH 00/80] TTY updates for 2.6.28 Alan Cox
2008-10-13  9:42 ` [PATCH 60/80] tty: extract the pty init time special cases Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®