Re: term & user space console

[Marcus Brinkmann]

On Wed, Jan 30, 2002 at 02:52:30PM -0800, Thomas Bushnell, BSG wrote:
> The Right Thing is surely two separate threads.  I'm not sure that
> select is really necessary at all however.  

I have stumbled upon an obscure race where it helps.  I am not sure if
it is worth the use of io_select, I will leave this for you to decide.
 
> The writer thread is similar:
> 
>   while (1)
>     if (data remains to be written)
>       write data -- blocking (sometimes, not often)
>     block until data needs to be written

Below is a snippet from the writer loop I have written so far.  Some side
issues have been cut out to make it shorter.

The issue I am talking about is implementing pending_output_size correctly,
which should return the number of bytes pending for output.  The problem
exists only for ioports that have the tiocoutq capability.  Because we don't
know if the underlying translator already has received the io_write or not,
we don't know what the correct return value is:

  int queue_size = 0;
  err = tioctl_tiocoutq (ioport, &queue_size);

  if (!err)
    return XXX_IO_WRITE_RECEIVED ? queue_size : queue_size + npending_output;

We fundamentally can not make this decision, there are too many races
against the, potentially blocking, io_write.

So now consider the contrary:  We use select.  This would mean that we wait
with io_select until the underlying translator is ready to accept data
without blocking.  Then we take the global lock and write in non-blocking
mode as much data as we can (until the queue is empty or get EWOULDBLOCK).
Then we release the global lock and go back to io_select.

Because we would hold the global lock during all write operations, there are
no pending write operations to race against.  The implementation of
pending_output_queue would always be:

    return queue_size + npending_output;

I could also imagine a solution which would try io_select and fall back to
the other solution if io_select is not implemented by the underlying node,
if that is of any concern.

Now, after the signature, an excerpt from the current implementation of the
writer loop.  What do you think?

Thanks,
Marcus

  mutex_lock (&global_lock);
  while (1)
    {
      while (!qsize (outputq) || (termflags & USER_OUTPUT_SUSP))
        hurd_condition_wait (&hurdio_writer_condition, &global_lock);

[...]

      /* Copy characters onto PENDING_OUTPUT, not bothering
         those already there. */
      size = qsize (outputq);
      if (size + npending_output > BUFFER_SIZE)
        size = BUFFER_SIZE - npending_output;

      bufp = pending_output + npending_output;
      npending_output += size;
      /* We need to save this value, as otherwise there might be a
         race with hurdio_abandon_physical_output, which might
         overwrite npending_output with zero after we release the
         lock.  */
      bytes_written = npending_output;

      while (size--)
        *bufp++ = dequeue (outputq);

      /* Submit all the outstanding characters to the I/O port. */
      mutex_unlock (&global_lock);
      err = io_write_request (ioport, pending_output, bytes_written,
                              -1, &amount);
      mutex_lock (&global_lock);

[... some error checking removed ...]

        if (!err)
        {
          /* Note that npending_output might be set to null in the
             meantime by hurdio_abandon_physical_output.  */
          if (amount >= npending_output)
            {
              npending_output = 0;
              condition_broadcast (outputq->wait);
            }
          else
            {
              /* Copy the characters that didn't get output
                 to the front of the array.  */
              npending_output -= amount;
              memmove (data, data + amount, datalen);
            }
[...]

    }

-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org brinkmd@debian.org
Marcus Brinkmann              GNU    http://www.gnu.org    marcus@gnu.org
Marcus.Brinkmann@ruhr-uni-bochum.de
http://www.marcus-brinkmann.de