Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751708AbbLNAQo (ORCPT ); Sun, 13 Dec 2015 19:16:44 -0500 Received: from mail-pf0-f181.google.com ([209.85.192.181]:35306 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751383AbbLNAQl (ORCPT ); Sun, 13 Dec 2015 19:16:41 -0500 Subject: Re: [PATCH v2 0/4] Replace tty->closing To: Greg Kroah-Hartman References: <1447020173-32207-1-git-send-email-peter@hurleysoftware.com> <1447071353-2961-1-git-send-email-peter@hurleysoftware.com> Cc: Jiri Slaby , linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, netdev@vger.kernel.org, Johannes Stezenbach , Karsten Keil , Martin Schwidefsky , Heiko Carstens , Jesper Nilsson , Mikael Starvik , Jiri Kosina , David Sterba , Mark Hounschell From: Peter Hurley Message-ID: <566E0A64.4080509@hurleysoftware.com> Date: Sun, 13 Dec 2015 16:16:36 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <1447071353-2961-1-git-send-email-peter@hurleysoftware.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 Content-Length: 5058 Lines: 119 Greg, Would you drop these 4 patches from tty-testing please? On 11/09/2015 04:15 AM, Peter Hurley wrote: > Hi Greg, > > This series cleans up a messy and poorly documented mechanism required > at tty final close to prevent drivers from crashing after h/w shutdown. > > Without special handling, N_TTY echoing will cause driver i/o requests > _after_ h/w shutdown, which typically crashes the driver. Currently, > the tty_struct::closing flag triggers this special handling. However, > this mechanism is error-prone and subject to driver misuse. > > This series replaces tty->closing with a ldisc-specific interface, > tty_ldisc_closing(), and implements this interface for N_TTY. > For tty drivers which use tty_port_close_start(), this change eliminates > the last vestige of direct driver<->ldisc interaction. The few tty drivers > which open-code the close() method [1] still use tty_ldisc_closing() directly. > > The tty driver is aware final close for the tty has commenced because the > tty->count == 1 in the close() method. On final close, the following is > also true: > 1. port->count == 1. tty drivers which ref count the port, use the > --port->count == 0 as a substitute condition for final close. I overlooked the implications of a blocked open here. Since a blocked open drops the port count while blocking, a port count of 1 is not equivalent to a tty count of 1 at tty_release() time. So even though the port is shutting down, the extra tty count held by the blocking open will keep the ldisc instance from being destructed; iow, a port count of 1 does _not_ imply final tty close. For the blocked open itself, this would not be a problem because the open will error out anyway, drop the tty count and cause final close. (And, oddly, trigger a second port shutdown which I need to consider the implications of.) However, there is a very small race window where a third process might be able to successfully reopen the tty, but now would have a dead-stick ldisc instance (because this series does not reset the ldisc instance back to the not closing state). Regards, Peter Hurley > 2. final close is occurring as a result of the last in-use file descriptor > release. Consequently, there will be no read/poll/ioctl in-progress. > 3. the line discipline instance will be stopped and destroyed immediately > after the tty driver completes the close() method > 4. the tty itself will be unrefed immediately after the line discipline > instance is destroyed. > > Thus, the ldisc and tty buffers need only be flushed once, as any data > received by the tty driver after the flush but before h/w shutdown will > be deleted when the line discipline instance is destroyed. > No new echoes will occur after the ldisc flush because the echo buffer > is also flushed and new input (which otherwise might generate echoes) is > ignored while closing. This series removes the extra tty_ldisc_flush() > being performed by most drivers after h/w shutdown. > > Additionally, the ldisc closing state need not be reset since the line > discipline instance is being destroyed, so no interface is provided to reset > closing. > > > [1] tty drivers which open-code the close() method > drivers/staging/dgnc/dgnc_tty.c > drivers/staging/dgap/dgap.c > drivers/tty/hvc/hvsi.c > drivers/tty/serial/68328serial.c > drivers/tty/serial/crisv10.c > drivers/isdn/i4l/isdn_tty.c > drivers/s390/char/con3215.c > > Changes to v2: > * Fixed tty_ldisc_closing() ld use found by Johannes Stezenbach > > > Regards, > > Peter Hurley (4): > tty: rocket: Remove private close_wait > n_tty: Ignore all read data when closing > tty: Abstract and encapsulate tty->closing behavior > tty: Remove drivers' extra tty_ldisc_flush() > > drivers/char/pcmcia/synclink_cs.c | 3 --- > drivers/isdn/i4l/isdn_tty.c | 2 +- > drivers/s390/char/con3215.c | 3 +-- > drivers/staging/dgap/dgap.c | 4 +--- > drivers/staging/dgnc/dgnc_tty.c | 4 +--- > drivers/tty/amiserial.c | 2 -- > drivers/tty/hvc/hvsi.c | 2 +- > drivers/tty/ipwireless/tty.c | 1 - > drivers/tty/n_tty.c | 15 +++++++++++---- > drivers/tty/rocket.c | 5 ----- > drivers/tty/rocket_int.h | 1 - > drivers/tty/serial/68328serial.c | 3 +-- > drivers/tty/serial/crisv10.c | 3 +-- > drivers/tty/serial/serial_core.c | 3 --- > drivers/tty/synclink.c | 1 - > drivers/tty/synclink_gt.c | 1 - > drivers/tty/synclinkmp.c | 1 - > drivers/tty/tty_ldisc.c | 20 ++++++++++++++++++++ > drivers/tty/tty_port.c | 5 +---- > include/linux/tty.h | 2 +- > include/linux/tty_ldisc.h | 9 +++++++++ > 21 files changed, 49 insertions(+), 41 deletions(-) > -- 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/