All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Jiri Slaby <jslaby@suse.cz>
Cc: gregkh@suse.de, alan@linux.intel.com,
	Linux kernel mailing list <linux-kernel@vger.kernel.org>
Subject: Re: patch "TTY: remove tty_locked" added to tty tree
Date: Wed, 24 Aug 2011 13:20:47 +0200	[thread overview]
Message-ID: <201108241320.47635.arnd@arndb.de> (raw)
In-Reply-To: <4E54C4ED.3060809@suse.cz>

On Wednesday 24 August 2011, Jiri Slaby wrote:
> On 08/24/2011 10:46 AM, Arnd Bergmann wrote:
> > On Tuesday 23 August 2011 20:54:08 Jiri Slaby wrote:
> >> On 08/23/2011 08:46 PM, Arnd Bergmann wrote:

> > I think I just saw another problem: uart_close takes port->mutex while
> > holding the BTM, then calls tty_wait_until_sent(). If this releases
> > and reaquires the BTM, you get an AB-BA deadlock with port->mutex.
> 
> Aargh, right. The question is why uart_close takes port->mutex there? It
> may take it even right before uart_shutdown. As tty_wait_until_sent (or
> uart_wait_until_sent) may be called e.g. from set_termios without that
> lock anyway. There are ->tx_empty and ->stop_rx that may need some
> protection. But those are register accessors, so they should be
> protected by some spinlock to not race with interrupts. Actually stop_rx
> is. And empty_rx is only in 8250.
> 
> And I don't see anything else there which would need be protected by the
> lock. Do you?

I have not looked at correctness of port->lock before, I just tried to
make sure that BTM correctly nests around it when I removed the BKL.

It's not clear to me what state->mutex protects in the serial_core, but
it has been around forever (used to be called state->sem) and is held in
all uart functions, which is at least consistent. IIRC what Alan's plan
for this was, uart_close should eventually get changed to use
tty_port_close_start or even tty_port_close. Maybe the time for that has
come now, lacking better alternatives?

A lot of other drivers call tty_port_close_start() before taking port->mutex.

	Arnd

  reply	other threads:[~2011-08-24 11:20 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <13141210141189@kroah.org>
2011-08-23 18:33 ` patch "TTY: remove tty_locked" added to tty tree Jiri Slaby
2011-08-23 18:46   ` Arnd Bergmann
2011-08-23 18:54     ` Jiri Slaby
2011-08-24  8:46       ` Arnd Bergmann
2011-08-24  9:31         ` Jiri Slaby
2011-08-24 11:20           ` Arnd Bergmann [this message]
2011-08-24 11:47             ` Jiri Slaby
2011-08-24 14:35               ` Arnd Bergmann
2011-08-24 14:35               ` Arnd Bergmann
2011-08-24 21:27                 ` Jiri Slaby
2011-08-24 21:42                   ` Greg KH
2011-08-24 21:42                     ` Greg KH
2011-08-24 21:48                     ` Jiri Slaby
2011-08-24 21:48                     ` Jiri Slaby
2011-08-24 21:54                       ` Greg KH
2011-08-24 21:54                         ` Greg KH
2011-08-25 13:12                   ` [PATCH 1/5] TTY: serial, use ASYNCB_CLOSING in uart_close Jiri Slaby
2011-08-25 13:12                     ` [PATCH 2/5] TTY: serial, move locking " Jiri Slaby
2011-08-25 13:12                     ` [PATCH 3/5] TTY: define tty_wait_until_sent_from_close Jiri Slaby
2011-08-25 13:12                     ` [PATCH 4/5] TTY: use tty_wait_until_sent_from_close in tty_port_close_start Jiri Slaby
2011-08-25 13:12                     ` [PATCH 5/5] TTY: use tty_wait_until_sent_from_close in other drivers Jiri Slaby
2011-08-25 15:15                     ` [PATCH 1/5] TTY: serial, use ASYNCB_CLOSING in uart_close Arnd Bergmann
2011-08-24 21:27                 ` patch "TTY: remove tty_locked" added to tty tree Jiri Slaby
2011-08-24 15:53             ` Alan Cox

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=201108241320.47635.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=alan@linux.intel.com \
    --cc=gregkh@suse.de \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.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.