implement put_char() in cdc-acm

implement put_char() in cdc-acm

[Oliver Neukum]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

Hi,

the theory that the lack of support for put_char() is a
major contributor to character loss in cdc-acm can be tested.
Sven, could you test the attached patch? It implements
the support.

	Regards
		Oliver


[-- Attachment #2: 0001-cdc-acm-implement-put_char-and-flush_chars.patch --]
[-- Type: text/x-patch, Size: 3292 bytes --]

From f3871a76d7e2876b0e6ad66aee91085ecf9b2785 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 21 Oct 2015 12:10:07 +0200
Subject: [PATCH] cdc-acm: implement put_char() and flush_chars()

This should cut down latencies and waste if the tty layer writes single bytes.

Signed-off-by: Oliver Neukum >oneukum@suse.com>
---
 drivers/usb/class/cdc-acm.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 68 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b30e742..1cb3124 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -712,9 +712,20 @@ static int acm_tty_write(struct tty_struct *tty,
 	}
 
 	if (acm->susp_count) {
+		if (acm->putbuffer) {
+			/* now to preserve order */
+			usb_anchor_urb(acm->putbuffer->urb, &acm->delayed);
+			acm->putbuffer = NULL;
+		}
 		usb_anchor_urb(wb->urb, &acm->delayed);
 		spin_unlock_irqrestore(&acm->write_lock, flags);
 		return count;
+	} else {
+		if (acm->putbuffer) {
+			/* at this point there is no good way to handle errors */
+			acm_start_wb(acm, acm->putbuffer);
+			acm->putbuffer = NULL;
+		}
 	}
 
 	stat = acm_start_wb(acm, wb);
@@ -725,6 +736,60 @@ static int acm_tty_write(struct tty_struct *tty,
 	return count;
 }
 
+static void acm_tty_flush_chars(struct tty_struct *tty)
+{
+	struct acm *acm = tty->driver_data;
+	struct acm_wb *cur = acm->putbuffer;
+	int err;
+	unsigned long flags;
+
+	acm->putbuffer = NULL;
+	err = usb_autopm_get_interface_async(acm->control);
+	spin_lock_irqsave(&acm->write_lock, flags);
+	if (err < 0) {
+		cur->use = 0;
+		goto out;
+	}
+
+	if (acm->susp_count)
+		usb_anchor_urb(cur->urb, &acm->delayed);
+	else
+		acm_start_wb(acm, cur);
+out:
+	spin_unlock_irqrestore(&acm->write_lock, flags);
+	return;
+}
+
+static int acm_tty_put_char(struct tty_struct *tty, unsigned char ch)
+{
+	struct acm *acm = tty->driver_data;
+	struct acm_wb *cur;
+	int wbn;
+	unsigned long flags;
+
+overflow:
+	cur = acm->putbuffer;
+	if (!cur) {
+		spin_lock_irqsave(&acm->write_lock, flags);
+		wbn = acm_wb_alloc(acm);
+		if (wbn >= 0) {
+			cur = &acm->wb[wbn];
+			acm->putbuffer = cur;
+		}
+		spin_unlock_irqrestore(&acm->write_lock, flags);
+		if (!cur)
+			return 0;
+	}
+
+	if (cur->len == acm->writesize) {
+		acm_tty_flush_chars(tty);
+		goto overflow;
+	}
+
+	cur->buf[cur->len++] = ch;
+	return 1;
+}
+
 static int acm_tty_write_room(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
@@ -1888,6 +1953,8 @@ static const struct tty_operations acm_ops = {
 	.cleanup =		acm_tty_cleanup,
 	.hangup =		acm_tty_hangup,
 	.write =		acm_tty_write,
+	.put_char =		acm_tty_put_char,
+	.flush_chars =		acm_tty_flush_chars,
 	.write_room =		acm_tty_write_room,
 	.ioctl =		acm_tty_ioctl,
 	.throttle =		acm_tty_throttle,
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index dd9af38..648a6f7 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -94,6 +94,7 @@ struct acm {
 	unsigned long read_urbs_free;
 	struct urb *read_urbs[ACM_NR];
 	struct acm_rb read_buffers[ACM_NR];
+	struct acm_wb *putbuffer;			/* for acm_tty_put_char() */
 	int rx_buflimit;
 	int rx_endpoint;
 	spinlock_t read_lock;
-- 
2.1.4

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

Hey Oliver,

On 27/10/15 16:07, Oliver Neukum wrote:
> the theory that the lack of support for put_char() is a
> major contributor to character loss in cdc-acm can be tested.
> Sven, could you test the attached patch? It implements
> the support.
Thanks a lot for caring about this. I applied the patch to a plain linux
4.2 tree, but unfortunately I couldn't see any improvements. Applying
the patch I originally sent to the list for comparison still fixed the
issue.

Best regards,
Sven



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Tue, 2015-10-27 at 16:45 +0100, Sven Brauch wrote:
> Hey Oliver,
> 
> On 27/10/15 16:07, Oliver Neukum wrote:
> > the theory that the lack of support for put_char() is a
> > major contributor to character loss in cdc-acm can be tested.
> > Sven, could you test the attached patch? It implements
> > the support.
> Thanks a lot for caring about this. I applied the patch to a plain linux
> 4.2 tree, but unfortunately I couldn't see any improvements. Applying

That is unfortunate. So your problem is with not enough buffers.

> the patch I originally sent to the list for comparison still fixed the
> issue.

Peter, what to do about this? I think there are issues with Sven's
original patch. In particular I can't see how it can guarantee
progress. Is there a way to just really increase tty buffers,
e.g. quadruple them?

Sven, could we run a few more tests before we look at your patch
again? It really has issues, but it does help and nothing else
does. If necessary I will merge an improved version of your patch.

	Regards
		Oliver


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

Re: implement put_char() in cdc-acm

[Peter Hurley]

On 10/28/2015 07:04 AM, Oliver Neukum wrote:
> On Tue, 2015-10-27 at 16:45 +0100, Sven Brauch wrote:
>> Hey Oliver,
>>
>> On 27/10/15 16:07, Oliver Neukum wrote:
>>> the theory that the lack of support for put_char() is a
>>> major contributor to character loss in cdc-acm can be tested.
>>> Sven, could you test the attached patch? It implements
>>> the support.
>> Thanks a lot for caring about this. I applied the patch to a plain linux
>> 4.2 tree, but unfortunately I couldn't see any improvements. Applying
> 
> That is unfortunate. So your problem is with not enough buffers.

Well, not necessarily.

As I wrote in the original thread, I changed the way input work is
scheduled which should impact Sven's use-case. Those patches are
on Greg's tty-next branch; afaik Sven has not tested with those.

Sven, please test Oliver's patch on that tree.
Also, please attach your target .config here.
Lastly, please confirm your test method/termios settings (iow, are
you using a reproducer or just 'cat big_file > /dev/ttyACM1')

>> the patch I originally sent to the list for comparison still fixed the
>> issue.
> 
> Peter, what to do about this?

1. Re-evaluate the input kworker latency with my patches in tty-next.
   If the max input kworker latency can be cut in 1/2, then Sven's use
   case will be comfortably in only 1/2 the tty buffer space.

   I have a quick howto doc in progress for measuring that scheduling
   lag with ftrace, and a simple awk script for processing that ftrace
   output to aid in determining why.

   This is the fundamental problem.

2. Fix unthrottle

   Ever since my work on firewire tty, I've known throttle/unthrottle
   needed rework, but the i/o path and locking problems were more
   pressing.

   This is near top of my TODO list.


> I think there are issues with Sven's
> original patch. In particular I can't see how it can guarantee
> progress. Is there a way to just really increase tty buffers,
> e.g. quadruple them?

Yes, this can be done per tty port (typically by the port driver
install() method). Code fragment below doubles the tty buffer ceiling:

	retval = tty_buffer_set_limit(port, 128 * 1024);
	if (retval)
		goto error....;


> Sven, could we run a few more tests before we look at your patch
> again? It really has issues, but it does help and nothing else
> does. If necessary I will merge an improved version of your patch.

I would much rather rework URB flow + unthrottle, as I previously
outlined in the original thread instead of introducing another
buffering layer.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Wed, 2015-10-28 at 08:23 -0400, Peter Hurley wrote:
> On 10/28/2015 07:04 AM, Oliver Neukum wrote:
> > On Tue, 2015-10-27 at 16:45 +0100, Sven Brauch wrote:
> >> Hey Oliver,
> >>
> >> On 27/10/15 16:07, Oliver Neukum wrote:

> > That is unfortunate. So your problem is with not enough buffers.
> 
> Well, not necessarily.
> 
> As I wrote in the original thread, I changed the way input work is
> scheduled which should impact Sven's use-case. Those patches are
> on Greg's tty-next branch; afaik Sven has not tested with those.
> 
> Sven, please test Oliver's patch on that tree.

It can be found at
https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/tty/

And yes, that is the much preferable option.

> Also, please attach your target .config here.
> Lastly, please confirm your test method/termios settings (iow, are
> you using a reproducer or just 'cat big_file > /dev/ttyACM1')
> 
> >> the patch I originally sent to the list for comparison still fixed the
> >> issue.
> > 
> > Peter, what to do about this?
> 
> 1. Re-evaluate the input kworker latency with my patches in tty-next.
>    If the max input kworker latency can be cut in 1/2, then Sven's use
>    case will be comfortably in only 1/2 the tty buffer space.
> 
>    I have a quick howto doc in progress for measuring that scheduling
>    lag with ftrace, and a simple awk script for processing that ftrace
>    output to aid in determining why.
> 
>    This is the fundamental problem.
> 
> 2. Fix unthrottle
> 
>    Ever since my work on firewire tty, I've known throttle/unthrottle
>    needed rework, but the i/o path and locking problems were more
>    pressing.
> 
>    This is near top of my TODO list.
> 
> 
> > I think there are issues with Sven's
> > original patch. In particular I can't see how it can guarantee
> > progress. Is there a way to just really increase tty buffers,
> > e.g. quadruple them?
> 
> Yes, this can be done per tty port (typically by the port driver
> install() method). Code fragment below doubles the tty buffer ceiling:
> 
> 	retval = tty_buffer_set_limit(port, 128 * 1024);
> 	if (retval)
> 		goto error....;
> 
> 
> > Sven, could we run a few more tests before we look at your patch
> > again? It really has issues, but it does help and nothing else
> > does. If necessary I will merge an improved version of your patch.
> 
> I would much rather rework URB flow + unthrottle, as I previously
> outlined in the original thread instead of introducing another
> buffering layer.

So would I. I merely am ready to do it as the very last resort.
Sven, could you do the test Peter indicated?
If his tty update and my patch still fail together, could you also
ramp up the tty buffer in the manner he described?

Peter, do you think I ought to upstream the support for put_char() ?

	Regards
		Oliver


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

Re: implement put_char() in cdc-acm

[Peter Hurley]

On 10/28/2015 08:33 AM, Oliver Neukum wrote:
> Peter, do you think I ought to upstream the support for put_char() ?

Yes, but I owe you a test jig to validate it first. I'll do my best
to get that to you today.

Regards,
Peter Hurley

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

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

Hey,

On 28/10/15 13:33, Oliver Neukum wrote:
>> Sven, please test Oliver's patch on that tree.
> It can be found at 
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/tty/
I finally got around to test this; sorry for the delay.

I cloned that tree (git describe said v4.3-rc5), and applied Oliver's
patch. With that kernel booted, the error occured much less frequently,
but was still reproducable under load. With 4.2.5, I get a failure after
a few hundred MB; with 4.3-rc5 plus patch, it transferred serveral gigabytes
without errors, but failed when I started compiling something in the
background as a test.

So, I guess, this improves things but there's still no guarantee that
the data sent by the device will actually be delivered.

Querying for the stty options while the program is running gives this:
# sudo stty -a -F /dev/ttyACM0
speed 9600 baud; rows 0; columns 0; line = 0;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>; eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O;
min = 0; time = 0;
-parenb -parodd -cmspar cs8 hupcl -cstopb cread clocal -crtscts
-ignbrk -brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr -icrnl -ixon -ixoff -iuclc -ixany -imaxbel -iutf8
-opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
-isig -icanon -iexten -echo -echoe -echok -echonl -noflsh -xcase -tostop -echoprt -echoctl -echoke -extproc

If there's anything else which would be helpful for you to have tested,
please let me know.

Best regards,
Sven


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Peter Hurley]

On 11/01/2015 02:28 PM, Sven Brauch wrote:
> Hey,
>
> On 28/10/15 13:33, Oliver Neukum wrote:
>>> Sven, please test Oliver's patch on that tree.
>> It can be found at
>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/tty/
> I finally got around to test this; sorry for the delay.
>
> I cloned that tree (git describe said v4.3-rc5), and applied Oliver's
> patch. With that kernel booted, the error occured much less frequently,
> but was still reproducable under load. With 4.2.5, I get a failure after
> a few hundred MB; with 4.3-rc5 plus patch, it transferred serveral gigabytes
> without errors, but failed when I started compiling something in the
> background as a test.
>
> So, I guess, this improves things but there's still no guarantee that
> the data sent by the device will actually be delivered.
>
> Querying for the stty options while the program is running gives this:
> # sudo stty -a -F /dev/ttyACM0
> speed 9600 baud; rows 0; columns 0; line = 0;
> intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>; eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O;
> min = 0; time = 0;
> -parenb -parodd -cmspar cs8 hupcl -cstopb cread clocal -crtscts
> -ignbrk -brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr -icrnl -ixon -ixoff -iuclc -ixany -imaxbel -iutf8
> -opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
> -isig -icanon -iexten -echo -echoe -echok -echonl -noflsh -xcase -tostop -echoprt -echoctl -echoke -extproc
>
> If there's anything else which would be helpful for you to have tested,
> please let me know.

Just the kernel .config of the host please.

Regards,
Peter Hurley

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

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1.1: Type: text/plain, Size: 138 bytes --]

On 01/11/15 20:59, Peter Hurley wrote:
> Just the kernel .config of the host please.
> 
> Regards,
> Peter Hurley

See attached.


[-- Attachment #1.2: config.gz --]
[-- Type: application/gzip, Size: 43158 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Sun, 2015-11-01 at 20:28 +0100, Sven Brauch wrote:
> I cloned that tree (git describe said v4.3-rc5), and applied Oliver's
> patch. With that kernel booted, the error occured much less
> frequently,
> but was still reproducable under load. With 4.2.5, I get a failure
> after
> a few hundred MB; with 4.3-rc5 plus patch, it transferred serveral
> gigabytes
> without errors, but failed when I started compiling something in the
> background as a test.

Hi,

given this test I will submit the patch.
Sven, can you ramp up the buffers in the manner Peter suggested
and retest? It looks like the throttling code isn't perfect yet,
but a larger buffer may make its performance good enough.

	Regards
		Oliver


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

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

On 02/11/15 12:32, Oliver Neukum wrote:
> given this test I will submit the patch.
Mind you, I don't know if the improvement is due to your patch or other
things. I didn't test the new kernel without your patch; should I?

> Sven, can you ramp up the buffers in the manner Peter suggested
> and retest? It looks like the throttling code isn't perfect yet,
> but a larger buffer may make its performance good enough.
I'll try as soon as possible, but I'm sceptical. That still doesn't
guarantee that no data is lost, which feels bad to me.

Thanks and best regards,
Sven


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Mon, 2015-11-02 at 21:27 +0100, Sven Brauch wrote:
> On 02/11/15 12:32, Oliver Neukum wrote:
> > given this test I will submit the patch.
> Mind you, I don't know if the improvement is due to your patch or other
> things. I didn't test the new kernel without your patch; should I?

If you find the time. But testing with enlarged buffers is
more important.

> > Sven, can you ramp up the buffers in the manner Peter suggested
> > and retest? It looks like the throttling code isn't perfect yet,
> > but a larger buffer may make its performance good enough.
> I'll try as soon as possible, but I'm sceptical. That still doesn't
> guarantee that no data is lost, which feels bad to me.

That cannot be guaranteed on a remote line. Basically
your device is special by doing implied flow control
when the host stops requesting data.

	Regards
		Oliver


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

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

On 03/11/15 09:58, Oliver Neukum wrote:
> Basically
> your device is special by doing implied flow control
> when the host stops requesting data.
Hmm, ok, interesting. But isn't that what always happens for USB bulk
transfers, which is always the transport layer for the driver in question?

Regards,
Sven


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Tue, 2015-11-03 at 11:13 +0100, Sven Brauch wrote:
> On 03/11/15 09:58, Oliver Neukum wrote:
> > Basically
> > your device is special by doing implied flow control
> > when the host stops requesting data.
> Hmm, ok, interesting. But isn't that what always happens for USB bulk
> transfers, which is always the transport layer for the driver in question?

Usual communications devices have a channel, like a phone line, on
the other side, through which data keeps coming. You cannot just
stop transferring from the device to the host and hope to preserve
all data.

	Regards
		Oliver


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

Re: implement put_char() in cdc-acm

[Peter Hurley]

On 11/02/2015 06:32 AM, Oliver Neukum wrote:
> It looks like the throttling code isn't perfect yet

Yeah, that's still a todo (ie., fixing the premature unthrottle).

Regards,
Peter Hurley

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

Re: implement put_char() in cdc-acm

[Sven Brauch]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

Hey,

On 28/10/15 13:23, Peter Hurley wrote:
> Sven, please test Oliver's patch on that tree.
I will do as soon as I get around to it, I hope on the weekend.

> Lastly, please confirm your test method/termios settings (iow, are
> you using a reproducer or just 'cat big_file > /dev/ttyACM1')
Sorry, what exactly do you mean by "reproducer"? I have a
microcontroller which acquires and transmits the data on the device end
of the USB connection. The data flows from device to host.

> I would much rather rework URB flow + unthrottle, as I previously
> outlined in the original thread instead of introducing another
> buffering layer.
From my non-kernel-dev point of view, this seems the way to go if the
strategy in my patch (technical flaws aside) is not acceptable.
Everything else, i.e. larger buffers or less delay, will certainly be a
welcome improvement but still does not guarantee data delivery.

A very similar patch, by the way, was already submitted a few years ago
[1] but not accepted for similar reasons as brought up here (I only
found that thread later on). That patch has a more elegant
implementation than mine, so you might prefer reviving that, if it
becomes relevant.

I will be happy to test any fixes which come up, although I can't
promise I can get around to do it immediately.

Thanks and best regards,
Sven

_______________
[1] https://marc.info/?l=linux-kernel&m=130754754705303&w=2


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: implement put_char() in cdc-acm

[Peter Hurley]

On 10/28/2015 11:53 AM, Sven Brauch wrote: 
> On 28/10/15 13:23, Peter Hurley wrote:
>> Sven, please test Oliver's patch on that tree.
> I will do as soon as I get around to it, I hope on the weekend.

Ok.

I'll get you the kworker scheduler latency profiling howto so
we can find out what accounts for the scheduling lag, if still causing
issues when you test that tree.

>> Lastly, please confirm your test method/termios settings (iow, are
>> you using a reproducer or just 'cat big_file > /dev/ttyACM1')
> Sorry, what exactly do you mean by "reproducer"? I have a
> microcontroller which acquires and transmits the data on the device end
> of the USB connection. The data flows from device to host.

Ok. (A reproducer in your case would be a _very simple_ host sink, and
maybe a software simulation of the device.)

Are you using an application to setup and log the host side?
Or are you just using the shell (eg., 'cat /dev/ttyACM1 > log_file')?

I ask because the termios settings on the host are relevant to the
problem. If you're using an application, then you can capture the termios
settings on the host side _while the app is acquiring_ with, eg.:

	sudo stty -a -F /dev/ttyACM1


>> I would much rather rework URB flow + unthrottle, as I previously
>> outlined in the original thread instead of introducing another
>> buffering layer.
> From my non-kernel-dev point of view, this seems the way to go if the
> strategy in my patch (technical flaws aside) is not acceptable.
> Everything else, i.e. larger buffers or less delay, will certainly be a
> welcome improvement but still does not guarantee data delivery.
> 
> A very similar patch, by the way, was already submitted a few years ago
> [1] but not accepted for similar reasons as brought up here (I only
> found that thread later on). That patch has a more elegant
> implementation than mine, so you might prefer reviving that, if it
> becomes relevant.

I've done similar things on very-high-speed serial connections as
a stopgap, but I'd really like to solve this within the tty layer
rather than by each very-high-speed driver.


> I will be happy to test any fixes which come up, although I can't
> promise I can get around to do it immediately.

Thanks again.

Regards,
Peter Hurley

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

Re: implement put_char() in cdc-acm

[Oliver Neukum]

On Wed, 2015-10-28 at 16:53 +0100, Sven Brauch wrote:
> > I would much rather rework URB flow + unthrottle, as I previously
> > outlined in the original thread instead of introducing another
> > buffering layer.
> From my non-kernel-dev point of view, this seems the way to go if the
> strategy in my patch (technical flaws aside) is not acceptable.
> Everything else, i.e. larger buffers or less delay, will certainly be
> a
> welcome improvement but still does not guarantee data delivery.

It just isn't the task of a driver to detect that we are running out of
buffer space. It would be interesting to know how we can tell the upper
layers how much data we can have in flight, but basically that
information should be used by a common algorithm so we don't need to
reinvent the wheel multiple times.

	Regards
		Oliver


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