Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752806AbdGHJBS (ORCPT ); Sat, 8 Jul 2017 05:01:18 -0400 Received: from mail-wr0-f194.google.com ([209.85.128.194]:33281 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752636AbdGHJBR (ORCPT ); Sat, 8 Jul 2017 05:01:17 -0400 Date: Sat, 8 Jul 2017 10:01:13 +0100 From: Okash Khawaja To: Greg Kroah-Hartman Cc: Jiri Slaby , Samuel Thibault , Alan Cox , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Kirk Reiser , Chris Brannon , speakup@linux-speakup.org Subject: Re: tty contention resulting from tty_open_by_device export Message-ID: <20170708090113.GA24331@sanghar> References: <20170707202841.GA4177@sanghar> <20170708083803.GA23080@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170708083803.GA23080@kroah.com> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1898 Lines: 58 On Sat, Jul 08, 2017 at 10:38:03AM +0200, Greg Kroah-Hartman wrote: > > + > > + if (tty) { > > + tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */ > > Put the comment above the line? > Sure > > + tty = ERR_PTR(-EBUSY); > > + } else { /* Returns with the tty_lock held for now */ > > I don't understand this comment. > This is same comment in tty_open_by_driver. Basically, tty_init_dev returns tty locked so that it doesn't dissapear from underneath the caller. I am not sure about the "for now" part of the comment. I'll remove it. > > + tty = tty_init_dev(driver, index); > > + set_bit(TTY_KOPENED, &tty->flags); > > + } > > +out: > > + mutex_unlock(&tty_mutex); > > + tty_driver_kref_put(driver); > > + return tty; > > +} > > +EXPORT_SYMBOL(tty_kopen); > > EXPORT_SYMBOL_GPL()? :) > Of course, will update > > /** > > * tty_open - open a tty device > > --- a/include/linux/tty.h > > +++ b/include/linux/tty.h > > @@ -362,6 +362,7 @@ struct tty_file_private { > > #define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver */ > > #define TTY_HUPPED 18 /* Post driver->hangup() */ > > #define TTY_LDISC_HALTED 22 /* Line discipline is halted */ > > +#define TTY_KOPENED 23 /* TTY exclusively opened by kernel */ > > Minor nit, please use tabs here. > Will do > Overall, the idea looks sane to me. Keeping userspace from opening a > tty that the kernel has opened internally makes sense, hopefully > userspace doesn't get too confused when that happens. I don't think we > normally return -EBUSY from an open call, have you seen what happens > with apps when you do this (like minicom?) > Yes, returning -EBUSY is a change in the interface. I will test against applications like minicom before resubmitting this. Thanks, Okash