Re: [PATCH v2 20/20] tty: Remove extra wakeup from pty write() path

From: Greg Kroah-Hartman
Date: Tue Jul 23 2013 - 11:01:41 EST


On Tue, Jul 23, 2013 at 08:57:44AM -0400, Peter Hurley wrote:
> On 07/20/2013 01:00 PM, Peter Hurley wrote:
> >On 06/15/2013 10:21 AM, Peter Hurley wrote:
> >>Acquiring the write_wait queue spin lock now accounts for the largest
> >>slice of cpu time on the tty write path. Two factors contribute to
> >>this situation; a overly-pessimistic line discipline write loop which
> >>_always_ sets up a wait loop even if i/o will immediately succeed, and
> >>on ptys, a wakeup storm from reads and writes.
> >>
> >>Writer wakeup does not need to be performed by the pty driver.
> >>Firstly, since the actual i/o is performed within the write, the
> >>line discipline write loop will continue while space remains in
> >>the flip buffers. Secondly, when space becomes avail in the
> >>line discipline receive buffer (and thus also in the flip buffers),
> >>the pty unthrottle re-wakes the writer (non-flow-controlled line
> >>disciplines unconditionally unthrottle the driver when data is
> >>received). Thus, existing in-kernel i/o is guaranteed to advance.
> >>Finally, writer wakeup occurs at the conclusion of the line discipline
> >>write (in tty_write_unlock()). This guarantees that any user-space write
> >>waiters are woken to continue additional i/o.
> >
> >Greg,
> >
> >I thought I should let you know I'm tracking down a bug/regression
> >related to this patch.
> >
> >In certain unusual pty/ldisc configurations, i/o fails to make
> >forward progress. I still stand by my commit message above, so I'm
> >in the process of instrumenting the i/o path so I can uncover the
> >cause of the failure.
>
> Mystery solved.
>
> [PATCH v4 23/24] n_tty: Special case pty flow control
> from the lockless n_tty receive path series introduced a regression
> in which i/o failed to advance.
>
> This only occurred when one end of a pty pair was set to an ldisc
> other than N_TTY. The special case optimization which that patch
> introduces failed to address that configuration.
>
> I've sent a v5 of that patch to resolve the regression.

Thanks for that, I'll work to queue all of your tty patches up today, as
they have been sitting out of the tree for too long :)

greg k-h
--
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/