Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1190016 > unrolled thread

Re: [PATCH] Fix data loss in cdc-acm

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] Fix data loss in cdc-acm Peter Hurley <peter@hurleysoftware.com> - 2015-07-22 16:40 +0200
    Re: [PATCH] Fix data loss in cdc-acm Oliver Neukum <oneukum@suse.com> - 2015-07-22 17:10 +0200
      Re: [PATCH] Fix data loss in cdc-acm Peter Hurley <peter@hurleysoftware.com> - 2015-08-05 02:30 +0200

#1190016 — Re: [PATCH] Fix data loss in cdc-acm

On 07/22/2015 04:40 AM, Oliver Neukum wrote:
> On Tue, 2015-07-21 at 12:45 -0400, Peter Hurley wrote:
>> Let me know if you need help instrumenting the tty buffers/throttling
>> to help figure out what the actual problem is.
>>
>> Regarding the patch itself, I have no opinion on the suitability of
>> simply not resubmitting urbs. However, that is exactly how the
>> throttle
>> mechanism works, and the tty buffer API is specifically designed to
>> allow drivers to manage flow via that interface as well (especially
>> for high-throughput drivers).
> 
> Could you please expand on how this is supposed to work?
> For once how does one learn that room is available again?

There are basically 3 mechanisms for managing rx data:
1. Allocate space when the data arrives; drop data if no space is avail
   and indicate buf_overrun. This is what most drivers do.
2. Allocate space when the data arrives; try to buffer uncopied data
   and resubmit the data later. Some high-throughput drivers (in the wild)
   do this (but less so now that the tty buffer space is configurable).
3. Pre-allocate space _before_ the data arrives (with tty_buffer_request_room());
   this is applicable to subsystems which know how much data can be in-flight
   at any one time. This guarantees that when rx data arrives buffer space is
   available (since it has already been allocated).

Drivers that use method 2 typically attempt to recopy the buffered data
when either new data arrives or @ unthrottle. I've seen others use deferred
work as well.

AFAIK no driver/subsystem is using method 3 for guaranteed delivery
of in-flight data, but it seems ideally suited to usb serial.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1190045

On Wed, 2015-07-22 at 10:30 -0400, Peter Hurley wrote:
> 3. Pre-allocate space _before_ the data arrives (with
> tty_buffer_request_room());
>    this is applicable to subsystems which know how much data can be
> in-flight
>    at any one time. This guarantees that when rx data arrives buffer
> space is
>    available (since it has already been allocated).
> 
> Drivers that use method 2 typically attempt to recopy the buffered
> data
> when either new data arrives or @ unthrottle. I've seen others use
> deferred
> work as well.
> 
> AFAIK no driver/subsystem is using method 3 for guaranteed delivery
> of in-flight data, but it seems ideally suited to usb serial.

Indeed. But flow control is still done by throttle/unthrottle, isn't it?

	Regards
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200325

On 07/22/2015 11:01 AM, Oliver Neukum wrote:
> On Wed, 2015-07-22 at 10:30 -0400, Peter Hurley wrote:
>> 3. Pre-allocate space _before_ the data arrives (with
>> tty_buffer_request_room());
>>    this is applicable to subsystems which know how much data can be
>> in-flight
>>    at any one time. This guarantees that when rx data arrives buffer
>> space is
>>    available (since it has already been allocated).
>>
>> Drivers that use method 2 typically attempt to recopy the buffered
>> data
>> when either new data arrives or @ unthrottle. I've seen others use
>> deferred
>> work as well.
>>
>> AFAIK no driver/subsystem is using method 3 for guaranteed delivery
>> of in-flight data, but it seems ideally suited to usb serial.
> 
> Indeed. But flow control is still done by throttle/unthrottle, isn't it?

Oliver, somehow I never saw this. Please accept my apologies.

Well, input flow control from the driver would be implicitly limited
by a failure to allocate new buffer space after the old buffer was
inserted and pushed. In the case of urbs, this would simply mean that
the urb could not be resubmitted yet, and would have to be deferred.
So preventing buffer overrun wouldn't rely on throttle.

WRT flow control, method 3 only needs to know when to resume.
As with method 2, this could be @ unthrottle or by retry in deferred work
(essentially polling). At least that's what's currently available.

Regardless, tty drivers should still respond to throttle/unthrottle
requests so as to limit input data when there is no active reader.

All that being said, unthrottle behaves degenerately at very high line
rates and throughput, which needs to be resolved.

The central issue is that the wrong thread is evaluating the wrong
conditions for restart. A further complication is that unthrottle
is guaranteed race-free reads of struct termios which limit the contexts
from which unthrottle can be called.

So to recap; the tty buffer api already provides the necessary interface
for preventing buffer overrun and drivers/subsystems with very high line
rates should be utilizing that interface instead of relying on throttle.

In addition, a more reliable/appropriate method of restart is required
(preferably by fixing when/why unthrottle is invoked). I'll look into that.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web