How important is it that tty_write_room doesn't lie?

How important is it that tty_write_room doesn't lie?

[Timur Tabi]

Greg,

As you may remember, I have a combination console/tty driver that talks to a
serial-like FIFO provided by our hypervisor.  Both tty_operations.write() and
console.write() talk to the same FIFO for character output.

I've implemented a tty_operations.write_room() function that queries the
hypervisor for amount of free space in the FIFO.  I've noticed a subtle bug in
this function, and I was hoping for some advice on how to fix it.

I believe I might have a problem when the following events happen in this order:

1) The TTY layer calls write_room() to determine the amount of free space in the
FIFO.

2) The *console* layer calls console.write() to write some data (e.g. the kernel
interrupted the TTY layer and executed a printk)

3) Control returns to the TTY layer, which calls tty_operations.write(),
assuming that the number returned by the previous call to write_room() is still
correct.

Because of the interjected console write, the FIFO no longer has a much room as
write_room() said it does.  When the TTY layer calls tty_operations.write(), my
driver returns a number smaller than the size of the buffer passed to (i.e. not
all characters were written).  The TTY layer, not expecting this, loses data.

Is my assessment of the situation correct?  If so, is there any way around this
problem that doesn't require implementing *two* software FIFOs in the driver:
one for the console interface, and one for the TTY interface?  If every driver
needs a FIFO like this, wouldn't it be simpler for the TTY and console layers to
provide their own FIFOs?  I feel like I'm missing something obvious.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How important is it that tty_write_room doesn't lie?

[Greg KH]

On Wed, Feb 23, 2011 at 01:56:41PM -0600, Timur Tabi wrote:
> Greg,
> 
> As you may remember, I have a combination console/tty driver that talks to a
> serial-like FIFO provided by our hypervisor.  Both tty_operations.write() and
> console.write() talk to the same FIFO for character output.
> 
> I've implemented a tty_operations.write_room() function that queries the
> hypervisor for amount of free space in the FIFO.  I've noticed a subtle bug in
> this function, and I was hoping for some advice on how to fix it.
> 
> I believe I might have a problem when the following events happen in this order:
> 
> 1) The TTY layer calls write_room() to determine the amount of free space in the
> FIFO.
> 
> 2) The *console* layer calls console.write() to write some data (e.g. the kernel
> interrupted the TTY layer and executed a printk)
> 
> 3) Control returns to the TTY layer, which calls tty_operations.write(),
> assuming that the number returned by the previous call to write_room() is still
> correct.
> 
> Because of the interjected console write, the FIFO no longer has a much room as
> write_room() said it does.  When the TTY layer calls tty_operations.write(), my
> driver returns a number smaller than the size of the buffer passed to (i.e. not
> all characters were written).  The TTY layer, not expecting this, loses data.

Um, the tty layer should not loose data, but it is hard, if not
impossible, to re-write from within the tty layer due to lots of
different issues (if the data came from within it's hard, if it came
from userspace, it should be fine.)

> Is my assessment of the situation correct?  If so, is there any way around this
> problem that doesn't require implementing *two* software FIFOs in the driver:
> one for the console interface, and one for the TTY interface?  If every driver
> needs a FIFO like this, wouldn't it be simpler for the TTY and console layers to
> provide their own FIFOs?  I feel like I'm missing something obvious.

I think that people don't normally hit this as the console code isn't
used as a tty and a console at the same time, right?

How big is your buffer in your FIFO?  Can you always just say you have a
smaller ammount in order to try to work around the tty layer trying to
send you a few extra bytes at times?

thanks,

greg k-h

Re: How important is it that tty_write_room doesn't lie?

[Timur Tabi]

Greg KH wrote:
> I think that people don't normally hit this as the console code isn't
> used as a tty and a console at the same time, right?

That's another thing I never understood.  It's rare for a driver to support both
the console and tty layers.  The serial core driver does that, but I can't find
any other examples.  I would think that a driver would support both interfaces,
because both are needed.  Simplistically, printk --> console, and printf -->
tty.  When would ever want user-space support but not kernel support?

> How big is your buffer in your FIFO?  

The FIFO can vary, but it's probably at least 2KB it size.  At least, we hope to
able to set it to that size in the field.  Currently, we set it to 4KB.

> Can you always just say you have a
> smaller ammount in order to try to work around the tty layer trying to
> send you a few extra bytes at times?

How many bytes extra?  I don't even have any hard evidence that this is actually
happening, but a customer is reporting lost characters and this the only thing
we could come up with.

The serial drivers seem to have a software FIFO for the TTY interface, but none
for the console interface.  uart_write() puts the data into an internal circular
buffer, and then calls uart_start().  serial8250_console_write(), however,
writes directly to the hardware.  Is this what I should be doing?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How important is it that tty_write_room doesn't lie?

[Ted Ts'o]

On Wed, Feb 23, 2011 at 02:48:18PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > I think that people don't normally hit this as the console code isn't
> > used as a tty and a console at the same time, right?
> 
> That's another thing I never understood.  It's rare for a driver to
> support both the console and tty layers.  The serial core driver
> does that, but I can't find any other examples.  I would think that
> a driver would support both interfaces, because both are needed.
> Simplistically, printk --> console, and printf --> tty.  When would
> ever want user-space support but not kernel support?

When you are supporting a large bank of modems?  A number of the
serial cards were originally created to support 32, 64, 128 serial
ports per PCI board, specifically to drive modems, in the bad old days
of modems.  :-)

And I suspect many modern serial devices are just 8250/16550A based,
so they get the console and userspace serial for free.  A large number
of the more exotic boards were created because the 16550A uart isn't
ideal if you are driving vast number of modems at the same time.

> The FIFO can vary, but it's probably at least 2KB it size.  At
> least, we hope to able to set it to that size in the field.
> Currently, we set it to 4KB.

Wow, the FIFO has gotten a lot larger than I ever remember them
getting even when people were doing 460kbps.  I'm guessing this is
because you're trying to defer interrupts for power saving reasons,
yes?  I was used to seeing FIFO sizes more in the 32-128 bytes, tops.  :-)

      	    	    	   	      - Ted

Re: How important is it that tty_write_room doesn't lie?

[Theodore Tso]

On Feb 23, 2011, at 5:57 PM, Ted Ts'o wrote:

>> The FIFO can vary, but it's probably at least 2KB it size.  At
>> least, we hope to able to set it to that size in the field.
>> Currently, we set it to 4KB.
> 
> Wow, the FIFO has gotten a lot larger than I ever remember them
> getting even when people were doing 460kbps.  I'm guessing this is
> because you're trying to defer interrupts for power saving reasons,
> yes?  I was used to seeing FIFO sizes more in the 32-128 bytes, tops.  :-)

One more thought: If your FIFO is that large, you might want to consider
simply having the interrupt driver wake up a kernel thread, say when it
half full (to give the scheduler time to wake up and let the kernel thread
run), and dequeue the FIFO in a process context.

Or as an intermediate step, do the FIFO dequeue in a bottom-half
handler, where you'll at least be able to do memory allocations.

Don't assume that you have to empty the entire FIFO into a circular
buffer in an interrupt handler, just because that's what the 8250/16550
driver does.  It was designed that way because it had very small FIFO's,
but there are other ways that serial drivers can work.

One device driver I worked on, for the Comtrol Rocketport, was designed
for a very large number of ports (32-128 serial ports) and it had a single 
huge buffer shared between all of the ports, and once an interrupt
kicked things off, it was more efficient to simply be constantly pulling
things out of the buffer and dispatching it to the various tty ports,
and in practice it was faster and more efficient never to pull
the characters out in interrupt context, but instead use a polling
type system instead.

Think of it as a very early form of NAPI interrupt mitigation, but for
serial ports instead of high speed ethernet.  :-)

-- Ted

Re: How important is it that tty_write_room doesn't lie?

[Greg KH]

On Wed, Feb 23, 2011 at 02:48:18PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > I think that people don't normally hit this as the console code isn't
> > used as a tty and a console at the same time, right?
> 
> That's another thing I never understood.  It's rare for a driver to support both
> the console and tty layers.  The serial core driver does that, but I can't find
> any other examples.  I would think that a driver would support both interfaces,
> because both are needed.  Simplistically, printk --> console, and printf -->
> tty.  When would ever want user-space support but not kernel support?

usb-serial devices usually never care about kernel support, and as Ted
posted, the majority of people use serial ports for modems.

> > How big is your buffer in your FIFO?  
> 
> The FIFO can vary, but it's probably at least 2KB it size.  At least, we hope to
> able to set it to that size in the field.  Currently, we set it to 4KB.

That's huge.

> > Can you always just say you have a
> > smaller ammount in order to try to work around the tty layer trying to
> > send you a few extra bytes at times?
> 
> How many bytes extra?  I don't even have any hard evidence that this is actually
> happening, but a customer is reporting lost characters and this the only thing
> we could come up with.

With such a large FIFO, I would be very surprised for overruns like
this, unless you are working at very slow baud rates.

> The serial drivers seem to have a software FIFO for the TTY interface, but none
> for the console interface.  uart_write() puts the data into an internal circular
> buffer, and then calls uart_start().  serial8250_console_write(), however,
> writes directly to the hardware.  Is this what I should be doing?

Try it out and see what happens :)

thanks,

greg k-h

Re: How important is it that tty_write_room doesn't lie?

[Alan Cox]

> I think that people don't normally hit this as the console code isn't
> used as a tty and a console at the same time, right?

And also on most hardware that it doesn't actually matter.

The underlying logic is that printk output is important, and Linus has
always insisted that what matters is that it gets out, even with
collateral damage be that vt output mangling or serial losses.

If it's an actual problem in your environment you could do a bit of quiet
internal buffering (or report less than you really have if your fifo
queue is big)

Alan