From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754385AbbHEA1H (ORCPT ); Tue, 4 Aug 2015 20:27:07 -0400 Received: from mail-qg0-f52.google.com ([209.85.192.52]:36563 "EHLO mail-qg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751570AbbHEA1F (ORCPT ); Tue, 4 Aug 2015 20:27:05 -0400 Message-ID: <55C15851.8070003@hurleysoftware.com> Date: Tue, 04 Aug 2015 20:26:57 -0400 From: Peter Hurley User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Oliver Neukum CC: Johan Hovold , One Thousand Gnomes , Toby Gray , Sven Brauch , Linux Kernel Mailing List , linux-serial@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH] Fix data loss in cdc-acm References: <55AC1883.4050605@svenbrauch.de> <20150720172546.GF20628@localhost> <55AD38E5.1090807@svenbrauch.de> <20150721091838.GG20628@localhost> <55AE7714.80306@hurleysoftware.com> <1437554428.5445.0.camel@suse.com> <55AFA900.6040605@hurleysoftware.com> <1437577303.5445.7.camel@suse.com> In-Reply-To: <1437577303.5445.7.camel@suse.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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