Re: Is tty->receive_room no longer usable w/ SMP?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/13/2014 12:52 PM, Grant Edwards wrote:
On 2014-02-13, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
On 02/13/2014 12:38 AM, Grant Edwards wrote:
While this approach is possible, there is another way to look at
this problem:

The tty buffers are very deep (at least 64K+, as high as 128K).
If data can't be taken in but the sender has not yet been throttled,
then some error is occurring.

Not necessarily.

With modern UARTs, the way you throttle the sender is by not reading
data from the rx FIFO when there's nowhere to put that data.  When the
rx FIFO starts getting too full, the UART will send an XOFF or assirt
or de-assert RTS or DTR or whatever.

In order for that mechanism to work, you need to know when to stop
reading the rx FIFO.  That's what we had been using tty->receive_room
for, and now it appears I should be using tty_buffer_request_room()
for.

Rather than buffering data outside the tty buffers (because
eventually that will run out too causing a fifo overflow),

Not if flow control is enabled.  When you end up with data buffered in
the driver, the driver stops reading the rx FIFO, the rx FIFO fills
up, and the UART throttles the sender.  Once the buffered data has
been transferred, you start read the rx FIFO again and the UART
unthrottles the sender.  If flow control _isn't_ enable, and the
application isn't reading receive data, then there's going to be an
overflow error and data is lost no matter what approach you take. It
makes a lot more sense to let the UART do the overflow error detection
and discard the data than to do that in driver code.

Ok. I assumed your hardware didn't do this because otherwise you wouldn't
need to use the tty_flip_* interface directly because your driver would
be a UART miniport (ie., serial-core port).

just drop the current rx and signal a condition to insert a
TTY_OVERRUN at the beginning of the next rx (look at
drivers/staging/fwserial/fwserial.c:fwtty_rx().

That seems like writing driver code to emulate functions that already
exist in the UART hardware.  I'd much rather just use the UART for
that.

Again, I assumed your hardware didn't do this for you (which is why
I pointed you to an example of how to simulate it for hardware that
isn't a UART/doesn't auto-flow-control).

but don't use tty_buffer_space_avail() throttling part; that's only
for very high speeds where the tty buffers can overflow even before
the input processing worker runs).

The whole point of knowing when to stop reading the rx FIFO is to
_prevent_ data loss/overrun by allowing the UART to throttle the
sender before we get to the point where we've got nowhere to put data
we've read from the rx FIFO.

I must be missing something.

tty_buffer_request_room() returns a buffer suitable for data + flags;
you're just misreading the code.

Great!

That solves my problem.  I can call tty_buffer_request_room(), read
the indicated number of chars or char/flag pairs and then depend on
being able to call _either_ tty_insert_flip_string() or
tty_insert_flip_string_flags() to transfer the data I read.

Yep.

When the application stops reading data and buffers fill up,
tty_buffer_request_room() will tell me to stop reading data from the
UART and everything will work the way it should.

At least build against it; that's automatable.

Good point.  That would catch the obvious stuff with very little work.

I don't mean to be critical but I can't really see that development
model being sustainable.

We've been doing it for 20+ years (though I've only been involved for
the last 15).  We can't beat the Chinese on the price of boards, so we
had better beat them on support.

Just trying to be helpful. Please don't take my comments as an attack
on either your business model or your development processes; I don't
know enough about either to criticize.

There's no realistic way to single-source a driver across hundreds of
kernel versions.

Maybe not hundreds.  Our current drivers only support 2.6.24 and
later.  We can still support previous driver versions if required for
customers running older 2.6 kernels.  We officially stopped supporting
2.4 several years ago.

By "support", do you mean "add new features" or "workaround hardware bugs"?

What was still being "maintained" for your driver for a 2.4 kernel
(since any other active development there stopped years ago)?

We hadn't supported 2.4 for several years at that point -- I was just
illustrating how reluctant many customers are to upgrade kernels and
the infeasibility of depending on in-tree drivers to support customers
like that.

One way to manage an in-kernel driver is to only have it support a
minimum/common feature set. The advantage is you get automatic
patches when the core changes -- and I don't just mean tty core but
also locking, workqueues, etc -- which you can then pull into the
out-of-tree driver (still adhering to the license requirements, of
course). You just maintain the actual hardware changes/bug fixes.

A couple of our products do have in-tree drivers, but they've diverged
so far from the out-of-tree drivers that they aren't particularly
useful resources for maintaining the out-of-tree drivers.  It's
actually been more useful over the years to look at changes made to
other in-tree drivers.

You have to deal with superseding the in-tree driver with the out-of-tree
driver when both are present, but that's not insurmountable.

That doesn't seem to be an issue.  It's only been a problem a couple
times in the past couple decades when a customer has built a custom
configured kernel and configured the in-tree driver as built-in rather
than a module.

[ For my own edification, why is the driver not a serial mini-port? ]

I don't know what a serial mini-port is.  We recently transitioned two
of our drivers from being tty drivers to being serial-core drivers.
Is mini-port something different that the serial core?

No, same thing.

For another driver, it's still a tty driver because the serial-core
API just didn't fit the device well enough to make it work.  One issue
I recall is that our driver for that device needs to be able to cause
specific error returns for write() calls that are made when the
hardware is unavailable (and I think there are different errors
depending on why it's unavailable).

And this is the driver that uses tty_flip_* interface directly?

Regards,
Peter Hurley

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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux