Are calls to open()/close() serialized by tty layer?

Are calls to open()/close() serialized by tty layer?

[Grant Edwards]

Are calls to a tty driver's open()/close() methods serialized by the
tty layer?  If so, has this always been the case?

I'm working on converting an old tty driver over to using the "new"
tty_port_* functions and the standard "struct tty_port" port data
structure.

[I thought about converting it to a uart driver for serial_core, but
AFAICT, that API doesn't provide any way to return errors for
userspace open() and write() calls and would therefore break my
user-land API.]

>From looking at example tty drivers in the 3.7.2 source tree, it
appears that they assume calls to open()/close() are serialized.

The old driver I'm working on had its own internal locking to protect
race conditiions when incrementing and decrementing of a port's open
count.  That counter is being prelaced by tty->port->count, and in
existing in-kernel tty drivers it doesn't look like there is any
locking when that count is incremented during an open().

For example, from rocket.c:

   891	static int rp_open(struct tty_struct *tty, struct file *filp)
   892	{
[...]   
   924	        tty->driver_data = info;
   925	        tty_port_tty_set(port, tty);
   926	
   927	        if (port->count++ == 0) {
   928	                atomic_inc(&rp_num_ports_open);
   929	
   930	#ifdef ROCKET_DEBUG_OPEN
   931	                printk(KERN_INFO "rocket mod++ = %d...\n",
   932	                                atomic_read(&rp_num_ports_open));
   933	#endif
   934	        }

If calls to open()/close() aren't serialized, then line 927 would
cause a race condition...

-- 
Grant Edwards               grant.b.edwards        Yow! Everybody gets free
                                  at               BORSCHT!
                              gmail.com            

Re: Are calls to open()/close() serialized by tty layer?

[Alan Cox]

On Wed, 16 Jan 2013 20:07:49 +0000 (UTC)
Grant Edwards <grant.b.edwards@gmail.com> wrote:

> Are calls to a tty driver's open()/close() methods serialized by the
> tty layer?  If so, has this always been the case?

They are not fully serialized - never have been. A lot of the drivers were
always racy, some that have not been fully converted to the port helpers
still are.

> >From looking at example tty drivers in the 3.7.2 source tree, it
> appears that they assume calls to open()/close() are serialized.
> 
> The old driver I'm working on had its own internal locking to protect
> race conditiions when incrementing and decrementing of a port's open
> count.  That counter is being prelaced by tty->port->count, and in
> existing in-kernel tty drivers it doesn't look like there is any
> locking when that count is incremented during an open().

Use the tty_port helpers. They will also handle all the other detail you
need. The helpers themselves provide serialized activate/shutdown
callbacks on the port which are serialized against each other and hangups.

The port helpers will also ensure your refcounting is right, you don't
have open/close/hangup races and that you get the required POSIX
semantics.

The port helpers are designed so you can propogate your own error codes
either by wrapping them or by returning your own error code from
port->activate() which will be duly propogated to user space.

Alan

Re: Are calls to open()/close() serialized by tty layer?

[Grant Edwards]

On 2013-01-16, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Grant Edwards <grant.b.edwards@gmail.com> wrote:
>
>> Are calls to a tty driver's open()/close() methods serialized by the
>> tty layer?  If so, has this always been the case?
>
> They are not fully serialized - never have been. A lot of the drivers
> were always racy, some that have not been fully converted to the port
> helpers still are.

That's what it looked like it to me, but I wasn't sure.

>> [...]
>> The old driver I'm working on had its own internal locking to protect
>> race conditiions when incrementing and decrementing of a port's open
>> count.  That counter is being replaced by tty->port->count, and in
>> existing in-kernel tty drivers it doesn't look like there is any
>> locking when that count is incremented during an open().
>
> Use the tty_port helpers.

That was the plan...

> They will also handle all the other detail you need. The helpers
> themselves provide serialized activate/shutdown callbacks on the port
> which are serialized against each other and hangups.

Ah! None of the examples I looked at so far were using tty_port_open()
and the port activate method, and I had somehow missed noticing their
existence when I was looking through tty_port sources.

> The port helpers will also ensure your refcounting is right, you
> don't have open/close/hangup races and that you get the required
> POSIX semantics.
>
> The port helpers are designed so you can propogate your own error
> codes either by wrapping them or by returning your own error code
> from port->activate() which will be duly propogated to user space.

Cool, thanks!

Which of the tty drivers is the best example to follow when it comes
to use of the tty_port_ helpers?

It looks like the tty_port_ helpers only go back to 2.6.33, so
supporting kernels older than that will require some sort of private,
backported versions of the helpers.  Maybe I can talk management and
tech support into dropping support for kernels older than 2.6.33...

-- 
Grant Edwards               grant.b.edwards        Yow! I threw up on my
                                  at               window!
                              gmail.com