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>,
	linux-m68k@vger.kernel.org,
	Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: patch "TTY: remove tty_locked" added to tty tree
Date: Wed, 24 Aug 2011 16:35:13 +0200	[thread overview]
Message-ID: <201108241635.13502.arnd@arndb.de> (raw)
In-Reply-To: <4E54E4D9.6070802@suse.cz>

On Wednesday 24 August 2011, Jiri Slaby wrote:
> On 08/24/2011 01:20 PM, Arnd Bergmann wrote:
> > 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)
> 
> It was actually moved in uart_close back in 2003. Formerly (when there
> was only a coarse grained port_sem) it was right before uart_shutdown.
> But there were some flags to handle some races. I'm not sure whether the
> flags protected any race here though.

ok

> > 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?
> 
> Yes, I have such a patch in my queue. But it's not easy to get there.
> You may take a look at:
> http://decibel.fi.muni.cz/gitweb/?p=linux.git;a=shortlog;h=refs/heads/devel
> 
> But it's still far from ready. And yet, in the queue, I still have
> port->mutex locked before tty_port_close_start like it is now.

Ah, right. I still don't see why the port->mutex is or is not needed there,
and I think that's the main issue.

By comparison, getting *_wait_until_sent to be called without BTM seems
easy -- we know that all callers from ->close() hold it, while the ones
from ->ioctl() don't. We could have a helper like

void tty_wait_until_sent_from_close(struct tty_struct *tty, long timeout)
{
	tty_unlock(); /* tty->ops->close holds the BTM, drop it while waiting */
	tty_wait_until_sent(tty, timeout);
	tty_lock();
}

to deal with that, if only we can sort the lock ordering with port->mutex.

BTW, I saw that the three m68k serial port drivers (amiserial, 68328, 68360)
all call *_wait_until_sent with interrupts disabled, which is even more
broken.

	Arnd

  parent reply	other threads:[~2011-08-24 14:35 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
2011-08-24 11:47             ` Jiri Slaby
2011-08-24 14:35               ` Arnd Bergmann
2011-08-24 14:35               ` Arnd Bergmann [this message]
2011-08-24 21:27                 ` Jiri Slaby
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 15:53             ` patch "TTY: remove tty_locked" added to tty tree 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=201108241635.13502.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=alan@linux.intel.com \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@suse.de \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@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.