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

From: Peter Hurley
Date: Wed May 15 2013 - 19:11:07 EST


On 05/15/2013 03:48 PM, Joerg Roedel wrote:
(also adding Konrad)

On Wed, May 15, 2013 at 02:45:52PM -0400, Peter Hurley wrote:
"space left" is not honored when OPOST is clear, so it is not protected
in this case. IOW, tty->ops->write_room() is not called, so by-definition
there is "space left".

Okay, so "space left" has to do with something tty-layer internal and
does not mean potential output-buffers handled by the console-drivers.

Well, "space left" does mean 'potential output-buffers'. However,
without OPOST, there is no output flow control as implemented through
the write_room() method. The driver is expected to write as much as
it can and return how much it wrote.

Are you certain your stack trace takes you through this particular
invocation of tty->ops->write()? Could it be that the compiler has
inlined process_output_block() into n_tty_write() and that's what your
seeing?

I am sure that the backtrace pointed to that invocation. I looked up the
return-address from the stack-trace in the objdump and it pointed to
that line after that invocation.

Ok.

But that implies that OPOST has been cleared (termios changed) which
doesn't really make sense for a console, which is why I asked.

Can you attach the BUG report?
Are you certain OPOST is cleared? (output of stty -a -F </dev/xxxx>)

Havn't checked OPOST. It is also hard to do because all I have is the
BUG and the kernel binary. I have no direct access to the machine.

Is CONFIG_CONSOLE_POLL=y?

Will check.

Is this happening during boot or much later?

Much later. It actually happened on a 3.2 kernel on a machine that ran
for several 100 days already. After that happened the box just rebooted
into a new kernel. I also checked the git-log from 3.2 to now and didn't
found a fix, also the code looks pretty similar so I guess the bug is
still there.

But not the only path to __write_console().

For example, what serializes hvc_console_print() with hvc_write()
for the same console index?

You are right, that does not look to be protected from each other. The
hvc_write() function has a spin_lock. But that does not prevent
hvc_console_print() from calling the put_chars function too.

I'll look something more into that. There is definitly a problem when
__write_console is called concurrently.

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).

I have one question about the
tty-layer: Do the console drivers have to expect parallel calls to
ops->write()?

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).

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/