Re: [PATCH] tty: Add missing lock in n_tty_write()

From: Peter Hurley
Date: Fri May 17 2013 - 15:08:33 EST


On 05/17/2013 07:48 AM, Joerg Roedel wrote:
Hi Peter,

thanks for you explanations. They helped me to better understand what is
happening now.

On Wed, May 15, 2013 at 07:10:43PM -0400, Peter Hurley wrote:
On 05/15/2013 03:48 PM, Joerg Roedel wrote:

Agreed. Those functions look written for single-producer/single-consumer
i/o model. (That's why I asked about CONFIG_CONSOLE_POLL=y as well because
that doesn't look thread-safe either).

Ok, I checked that. CONFIG_CONSOLE_POLL is on in that kernel.

Just to be clear here: there's a difference between a console driver
and a tty driver.

The console driver's write() method is serialized with the global
console_lock() so parallel console writes are not possible.

No such guarantee exists for the tty driver write() method, although it
probably wouldn't be difficult to provide that guarantee (since the
line discipline write() is already serialized by tty->atomic_write_lock).

Okay, so it is safe to say that currently the drivers write() (and
put_chars()) functions need to expect to be called concurrently and
therefore they have to serialize themselves when they need it, right?

If only it were that simple :)

Yes, console write() and tty write() can be concurrent. However, the
console write() can also be _recursive_ wrt. tty write(). This can happen,
for example, if something oopses in the tty write() path.

If you review serial8250_console_write() in drivers/tty/serial/8250/8250_core.c,
you'll see how some of this is worked around.

But looking at this from a wider perspective, the goal should be
to limit the overlap as much as possible.

Regards,
Peter Hurley



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/