I have written a program that reads and writes data to the serial uart using
async io.
Unfortunately it does not work properly because a SIGIO is never gerated when
output becomes possible, the reason is that TTY_DO_WRITE_WAKEUP is not set for
most of the tty drivers. After i set the bit in serial_core.c the program works
as expected.
Now i wonder why TTY_DO_WRITE_WAKEUP is almost always disabled ?
Below is a test case.
Thank you in advance,
Thomas
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <string.h>
#include <stdlib.h>
#include <sys/fcntl.h>
#include <termios.h>
#include <errno.h>
static const char outbuf [] = "01234567890abcdef\n";
static void sigio_handler (int sig __attribute__((unused)))
{
}
static int sigact (int sig, void (*handler)(int))
{
struct sigaction sa;
memset (&sa, 0, sizeof (sa));
sa.sa_flags = SA_NOCLDWAIT | SA_NOCLDSTOP;
sigfillset (&sa.sa_mask);
sa.sa_handler = handler;
return sigaction (sig, &sa, NULL);
}
static int uart_set_termio (int fd)
{
struct termios term;
if (tcgetattr (fd, &term) == -1)
return -1;
term.c_cflag = B19200 | CS8 | CREAD | CLOCAL;
term.c_iflag = IGNPAR;
term.c_oflag = 0;
term.c_lflag = 0;
term.c_cc [VTIME] = 0;
term.c_cc [VMIN] = 0;
return tcsetattr (fd, TCSAFLUSH, &term);
}
static int uart_fd_init (int fd)
{
if (fcntl (fd, F_SETSIG, 0) == -1 ||
fcntl (fd, F_SETOWN, getpid ()) == -1)
return -1;
return fcntl (fd, F_SETFL, fcntl (fd, F_GETFL) | O_ASYNC | O_NONBLOCK);
}
static int uart_sig_init (void)
{
sigset_t sigset;
if (sigact (SIGIO, sigio_handler) == -1)
return -1;
if (sigemptyset (&sigset) == -1 ||
sigaddset (&sigset, SIGIO) == -1)
return -1;
return sigprocmask (SIG_BLOCK, &sigset, NULL);
}
int main (int argc, char *argv [])
{
int uart_fd;
sigset_t sigmask;
const int outbuflen = strlen (outbuf);
const char *dev = "/dev/ttyS0";
if (argc > 1)
dev = argv [1];
if (uart_sig_init () == -1)
return EXIT_FAILURE;
uart_fd = open (dev, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (uart_fd == -1)
return EXIT_FAILURE;
if (uart_set_termio (uart_fd) == -1 ||
uart_fd_init (uart_fd) == -1)
return EXIT_FAILURE;
if (sigprocmask (SIG_SETMASK, NULL, &sigmask) == -1 ||
sigdelset (&sigmask, SIGIO) == -1)
return EXIT_FAILURE;
while (1)
{
int written;
int towrite = outbuflen;
const char *buf = outbuf;
while (towrite)
{
written = write (uart_fd, buf, towrite);
if (written == -1)
{
if (errno == EAGAIN)
{
sigsuspend (&sigmask);
continue;
}
else
return EXIT_FAILURE;
}
buf += written;
towrite -= written;
}
}
return EXIT_SUCCESS;
}
On Thu, 20 Nov 2008 11:06:19 +0100 (CET)
Thomas Pfaff <[email protected]> wrote:
> I have written a program that reads and writes data to the serial uart using
> async io.
> Unfortunately it does not work properly because a SIGIO is never gerated when
> output becomes possible, the reason is that TTY_DO_WRITE_WAKEUP is not set for
> most of the tty drivers. After i set the bit in serial_core.c the program works
> as expected.
> Now i wonder why TTY_DO_WRITE_WAKEUP is almost always disabled ?
Almost nobody uses SIGIO based I/O, especially on serial so I'm not
actually suprised by this one. If you've got patches that fix it then
please send them in.
Alan
On Thu, 20 Nov 2008, Alan Cox wrote:
> On Thu, 20 Nov 2008 11:06:19 +0100 (CET)
> Thomas Pfaff <[email protected]> wrote:
>
> > I have written a program that reads and writes data to the serial uart using
> > async io.
> > Unfortunately it does not work properly because a SIGIO is never gerated when
> > output becomes possible, the reason is that TTY_DO_WRITE_WAKEUP is not set for
> > most of the tty drivers. After i set the bit in serial_core.c the program works
> > as expected.
> > Now i wonder why TTY_DO_WRITE_WAKEUP is almost always disabled ?
>
> Almost nobody uses SIGIO based I/O, especially on serial so I'm not
> actually suprised by this one. If you've got patches that fix it then
> please send them in.
Is the TTY_DO_WRITE_WAKEUP bit some kind of cruft that can be removed entirely
or should i generate a patch for serial devices only ?
Removing it seems to be cleaner to me but i am unsure if it will break some
other stuff.
> Is the TTY_DO_WRITE_WAKEUP bit some kind of cruft that can be removed entirely
> or should i generate a patch for serial devices only ?
It shouldn't happen at the serial level. The line discipline sets the
flag as and when it has data for the process above.
The expected flow in normal use is
app writes to tty
data goes to n_tty ldisc
goes to tty
n_tty ldisc seems tty blocked
n_tty ldisc sets DO_WRITE_WAKEUP
tty gets ucongested
calls n_tty write_wakeup
SIGIO sent
Flag set again
The important part of the logic is in n_tty.c and tty_io.c
Alan
On Thu, 20 Nov 2008, Alan Cox wrote:
> > Is the TTY_DO_WRITE_WAKEUP bit some kind of cruft that can be removed entirely
> > or should i generate a patch for serial devices only ?
>
> It shouldn't happen at the serial level. The line discipline sets the
> flag as and when it has data for the process above.
>
> The expected flow in normal use is
>
> app writes to tty
> data goes to n_tty ldisc
> goes to tty
> n_tty ldisc seems tty blocked
> n_tty ldisc sets DO_WRITE_WAKEUP
>
> tty gets ucongested
> calls n_tty write_wakeup
> SIGIO sent
> Flag set again
>
> The important part of the logic is in n_tty.c and tty_io.c
Following the source the TTY_DO_WRITE_WAKEUP flag is set in n_tty_write_wakeup.
But n_tty_write_wakeup is only called from tty_wakeup when this bit is already
set, therefore it makes no sense to set this bit in n_tty_write_wakeup again.
The flow looks to me as
If the tty driver sets TTY_DO_WRITE_WAKEUP a SIGIO is generated on every
tty_wakeup.
If it is not set then n_tty_write_wakeup is never called and a SIGIO is not
generated.
Thomas
> But n_tty_write_wakeup is only called from tty_wakeup when this bit is already
> set, therefore it makes no sense to set this bit in n_tty_write_wakeup again.
The base code should probably really use test_and_clear_bit() when
calling that method.
>
> The flow looks to me as
>
> If the tty driver sets TTY_DO_WRITE_WAKEUP a SIGIO is generated on every
> tty_wakeup.
>
> If it is not set then n_tty_write_wakeup is never called and a SIGIO is not
> generated.
Which isn't perfect (excess SIGIO cases) but doesn't seem incorrect. If
you've not blocked the tty output buffer then write() has not returned a
short write and no SIGIO is due.
Alan
On Thu, 20 Nov 2008, Alan Cox wrote:
> > But n_tty_write_wakeup is only called from tty_wakeup when this bit is already
> > set, therefore it makes no sense to set this bit in n_tty_write_wakeup again.
>
> The base code should probably really use test_and_clear_bit() when
> calling that method.
Why should you test it, clear it and set it again in n_tty_write_wakeup ?
> >
> > The flow looks to me as
> >
> > If the tty driver sets TTY_DO_WRITE_WAKEUP a SIGIO is generated on every
> > tty_wakeup.
> >
> > If it is not set then n_tty_write_wakeup is never called and a SIGIO is not
> > generated.
>
> Which isn't perfect (excess SIGIO cases) but doesn't seem incorrect. If
> you've not blocked the tty output buffer then write() has not returned a
> short write and no SIGIO is due.
>
Of course this is not incorrect, but this does not solve my problem with the
TTY_DO_WRITE_WAKEUP flag.
IMHO a SIGIO on write possible should always be generated if the user wants it,
currently it is generated when the user wants it and the tty driver enables the
TTY_DO_WRITE_WAKEUP flag. Unfortunately most drivers don't set it.
Regarding excess SIGIO cases:
Once a write fails with EAGAIN a flag can be set and only in that case a SIGIO is
generated, afterwards the bit is cleared. Maybe that is what TTY_DO_WRITE_WAKEUP
was intended for.
Thomas
> > The base code should probably really use test_and_clear_bit() when
> > calling that method.
>
> Why should you test it, clear it and set it again in n_tty_write_wakeup ?
Because it should only be set again if a wakeup is needed. If the fasync
list for the tty is now empty it should stay clear.
> IMHO a SIGIO on write possible should always be generated if the user wants it,
> currently it is generated when the user wants it and the tty driver enables the
> TTY_DO_WRITE_WAKEUP flag. Unfortunately most drivers don't set it.
It is nothing to do with the driver. The line discipline sets it - or
rather should set it. If you have a case where you get an EAGAIN or short
write and the line discipline is not setting it then that is what needs
fixing not the drivers.
> Once a write fails with EAGAIN a flag can be set and only in that case a SIGIO is
> generated, afterwards the bit is cleared. Maybe that is what TTY_DO_WRITE_WAKEUP
> was intended for.
Correct.
A quick further look suggests you need to add
if (tty->fasync && (b - buf) != nr)
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
at the bottom of n_tty_write just before the return statement.
Alan