All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
Cc: Johan Hovold <jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jiri Slaby <jslaby-AlSwsSmVLrQ@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Alan Stern
	<stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>,
	USB list <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [Fwd: [PATCH v2 0/4] TTY: port hangup and close fixes]
Date: Thu, 7 Mar 2013 10:43:20 +0100	[thread overview]
Message-ID: <20130307094320.GB23635@localhost> (raw)
In-Reply-To: <1362597296.18799.198.camel-AsKIXgLx6sE@public.gmane.org>

On Wed, Mar 06, 2013 at 02:14:56PM -0500, Peter Hurley wrote:
> On Wed, 2013-03-06 at 17:52 +0100, Johan Hovold wrote:
> 
> > > > @@ -225,15 +232,13 @@ void tty_port_hangup(struct tty_port *port)
> > >         spin_lock_irqsave(&port->lock, flags);
> > >         port->count = 0;
> > >         port->flags &= ~ASYNC_NORMAL_ACTIVE;
> > > -       if (port->tty) {
> > > +       if (port->tty)
> > >                 set_bit(TTY_IO_ERROR, &port->tty->flags);
> > > -               tty_kref_put(port->tty);
> > > -       }
> > > -       port->tty = NULL;
> > >         spin_unlock_irqrestore(&port->lock, flags);
> > > > +       tty_port_shutdown(port, port->tty);
> > > 
> > > What prevents port->tty to be NULL here already?
> > 
> > Nothing, I'll get a new reference within the port lock section as you
> > just suggested elsewhere in this thread.
> 
> Don't do that. Steal the tty and put the kref after like this:
 
Allright.

> > Yes, I did. First, the order should not matter for blocked opens as they
> > will exit their wait loops based on tty_hung_up_p(filp) either way.
> 
> Only if the open() was ever successful, otherwise the filp won't be in
> the tty->tty_files list. That's why the blocking opens also check
> ASYNC_INITIALIZED (or ASYNCB_INITIALIZED depending on which they use).
> Which is why I said it was actually better to shutdown() first, then
> wake up the blocked opens.

ASYNC_INITIALIZED have also been cleared when the blocked opens are
being woken up from tty_port_close_end.

And the filp is added to tty_files before open() is called:

===>    tty_add_file(tty, filp);

	check_tty_count(tty, __func__);
	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
	    tty->driver->subtype == PTY_TYPE_MASTER)
		noctty = 1;
#ifdef TTY_DEBUG_HANGUP
	printk(KERN_DEBUG "%s: opening %s...\n", __func__, tty->name);
#endif
	if (tty->ops->open)
===>		retval = tty->ops->open(tty, filp);

so a blocked open must have hung_up_tty_fops when woken up from hangup,
right?

Either way, postponing wake-up somewhat in tty_port_hangup should be
fine.

Thanks,
Johan
--
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

WARNING: multiple messages have this Message-ID (diff)
From: Johan Hovold <jhovold@gmail.com>
To: Peter Hurley <peter@hurleysoftware.com>
Cc: Johan Hovold <jhovold@gmail.com>, Jiri Slaby <jslaby@suse.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	USB list <linux-usb@vger.kernel.org>,
	linux-serial@vger.kernel.org, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [Fwd: [PATCH v2 0/4] TTY: port hangup and close fixes]
Date: Thu, 7 Mar 2013 10:43:20 +0100	[thread overview]
Message-ID: <20130307094320.GB23635@localhost> (raw)
In-Reply-To: <1362597296.18799.198.camel@thor.lan>

On Wed, Mar 06, 2013 at 02:14:56PM -0500, Peter Hurley wrote:
> On Wed, 2013-03-06 at 17:52 +0100, Johan Hovold wrote:
> 
> > > > @@ -225,15 +232,13 @@ void tty_port_hangup(struct tty_port *port)
> > >         spin_lock_irqsave(&port->lock, flags);
> > >         port->count = 0;
> > >         port->flags &= ~ASYNC_NORMAL_ACTIVE;
> > > -       if (port->tty) {
> > > +       if (port->tty)
> > >                 set_bit(TTY_IO_ERROR, &port->tty->flags);
> > > -               tty_kref_put(port->tty);
> > > -       }
> > > -       port->tty = NULL;
> > >         spin_unlock_irqrestore(&port->lock, flags);
> > > > +       tty_port_shutdown(port, port->tty);
> > > 
> > > What prevents port->tty to be NULL here already?
> > 
> > Nothing, I'll get a new reference within the port lock section as you
> > just suggested elsewhere in this thread.
> 
> Don't do that. Steal the tty and put the kref after like this:
 
Allright.

> > Yes, I did. First, the order should not matter for blocked opens as they
> > will exit their wait loops based on tty_hung_up_p(filp) either way.
> 
> Only if the open() was ever successful, otherwise the filp won't be in
> the tty->tty_files list. That's why the blocking opens also check
> ASYNC_INITIALIZED (or ASYNCB_INITIALIZED depending on which they use).
> Which is why I said it was actually better to shutdown() first, then
> wake up the blocked opens.

ASYNC_INITIALIZED have also been cleared when the blocked opens are
being woken up from tty_port_close_end.

And the filp is added to tty_files before open() is called:

===>    tty_add_file(tty, filp);

	check_tty_count(tty, __func__);
	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
	    tty->driver->subtype == PTY_TYPE_MASTER)
		noctty = 1;
#ifdef TTY_DEBUG_HANGUP
	printk(KERN_DEBUG "%s: opening %s...\n", __func__, tty->name);
#endif
	if (tty->ops->open)
===>		retval = tty->ops->open(tty, filp);

so a blocked open must have hung_up_tty_fops when woken up from hangup,
right?

Either way, postponing wake-up somewhat in tty_port_hangup should be
fine.

Thanks,
Johan

  parent reply	other threads:[~2013-03-07  9:43 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1362085054.3337.20.camel@thor.lan>
2013-03-05 16:02 ` [Fwd: [PATCH v2 0/4] TTY: port hangup and close fixes] Jiri Slaby
2013-03-05 17:06   ` Peter Hurley
     [not found]     ` <1362503170.18799.33.camel-AsKIXgLx6sE@public.gmane.org>
2013-03-05 21:56       ` Jiri Slaby
2013-03-05 21:56         ` Jiri Slaby
2013-03-05 22:02         ` Peter Hurley
2013-03-05 22:10           ` Jiri Slaby
2013-03-05 22:32             ` Peter Hurley
2013-03-06 16:23               ` Jiri Slaby
2013-03-06 16:52   ` Johan Hovold
2013-03-06 19:14     ` Peter Hurley
     [not found]       ` <1362597296.18799.198.camel-AsKIXgLx6sE@public.gmane.org>
2013-03-07  9:43         ` Johan Hovold [this message]
2013-03-07  9:43           ` Johan Hovold
2013-03-07 21:52           ` Peter Hurley
     [not found]   ` <51361724.4050107-AlSwsSmVLrQ@public.gmane.org>
2013-03-07 14:55     ` [PATCH v3 0/6] TTY: port hangup and close fixes Johan Hovold
2013-03-07 14:55       ` Johan Hovold
     [not found]       ` <1362668153-10972-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-03-07 14:55         ` [PATCH v3 1/6] TTY: clean up port shutdown Johan Hovold
2013-03-07 14:55           ` Johan Hovold
2013-03-07 14:55       ` [PATCH v3 2/6] TTY: wake up processes last at hangup Johan Hovold
2013-03-07 14:55       ` [PATCH v3 3/6] TTY: fix DTR being raised on hang up Johan Hovold
2013-03-13 19:43         ` Peter Hurley
     [not found]           ` <1363203823.25976.102.camel-AsKIXgLx6sE@public.gmane.org>
2013-03-15  9:24             ` Johan Hovold
2013-03-15  9:24               ` Johan Hovold
2013-03-15 11:03               ` Peter Hurley
2013-03-15 11:03                 ` Peter Hurley
2013-03-15 11:30                 ` Johan Hovold
2013-03-15 11:57                   ` Peter Hurley
2013-03-07 14:55       ` [PATCH v3 4/6] TTY: fix DTR not being dropped " Johan Hovold
2013-03-07 14:55       ` [PATCH v3 5/6] TTY: clean up port drain-delay handling Johan Hovold
2013-03-07 14:55       ` [PATCH v3 6/6] TTY: fix close of uninitialised ports Johan Hovold
2013-03-13 19:50       ` [PATCH v3 0/6] TTY: port hangup and close fixes Peter Hurley
2013-03-15  9:29         ` Johan Hovold
2013-03-15 19:05       ` Greg Kroah-Hartman
2013-03-15 19:42         ` Johan Hovold
2013-03-18 23:28       ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130307094320.GB23635@localhost \
    --to=jhovold-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=jslaby-AlSwsSmVLrQ@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.