linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Johan Hovold <jhovold@gmail.com>
Cc: Toby Gray <toby.gray@realvnc.com>,
	Oliver Neukum <oliver@neukum.name>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] USB: cdc-acm: Prevent data loss when filling tty buffer.
Date: Tue, 22 Mar 2011 10:35:34 +0000	[thread overview]
Message-ID: <20110322103534.2284099e@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <20110322100526.GB21343@localhost>

> re-submitted. So the only way this will work is if it can be guaranteed
> that the line discipline will throttle and later unthrottle us. I
> doubt that is the case, but perhaps Alan can give a more definite
> answer?

If an ldisc throttles it should always later unthrottle. However flow
control is async so at high enough data rates it'll be too slow.

> I would also prefer a more generic solution to the problem so that we
> don't need to re-introduce driver buffering again. Since we already have
> the throttelling mechanism in place, if we could only be notified/find
> out that the tty buffers are say half-full, we could throttle (from

The tty layer actually knows this fact

> within the driver) but still push the remaining buffer still on the wire
> as they arrive. It would of course require a guarantee that such a
> throttle-is-about-to-happen notification is actually followed by (a
> throttle and) unthrottle. Thoughts on that?

tty throttling is at the ldisc layer, the tty buffers are below this. The
space left being 64K - tty->buf.memory_used

So you can certainly add the following routine

int tty_constipated(struct tty *t)
{
	if (tty->buf.memory_used > 49152)
		return 1;
	return 0;
}
EXPORT_SYMBOL_GPL(tty_constipated);

to drivers/tty/tty_buffer.c

The wakeup side is a bit trickier.

The down side of this of course is that you are likely to run at below
peak performance as you'll keep throttling back the hardware, whereas if
you have a tickless kernel with HZ set to 1000 it's probably sufficient
to bump the buffer sizes.

Right now (see tty_buffer.c) it's simply set to 64K as a sanity check
against throttled devices not stopping, but there isn't actually any
reason it couldn't be configurable at port setup time.


  reply	other threads:[~2011-03-22 10:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-21 15:52 [PATCH] USB: cdc-acm: Prevent data loss when filling tty buffer Toby Gray
2011-03-21 16:56 ` Alan Cox
2011-03-21 17:58   ` Toby Gray
2011-03-22  8:07   ` Oliver Neukum
2011-03-22 11:07     ` Alan Cox
2011-03-21 18:04 ` [PATCH v2] " Toby Gray
2011-03-22 10:05   ` Johan Hovold
2011-03-22 10:35     ` Alan Cox [this message]
2011-03-22 11:34       ` Johan Hovold
2011-03-22 14:11     ` Toby Gray
2011-03-22 18:05       ` Alan Cox
2011-03-22  7:43 ` [PATCH] " Oliver Neukum
2011-03-23 12:15   ` Toby Gray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110322103534.2284099e@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@suse.de \
    --cc=jhovold@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=oliver@neukum.name \
    --cc=toby.gray@realvnc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).