All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Andreas Bombe <aeb@debian.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
	linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>
Subject: Re: tty_lock held during transmit wait in close: still unresolved
Date: Fri, 27 May 2011 13:38:54 +0200	[thread overview]
Message-ID: <201105271338.54713.arnd@arndb.de> (raw)
In-Reply-To: <20110527004104.GB4369@amos.fritz.box>

On Friday 27 May 2011, Andreas Bombe wrote:
> > At any point you can show the code sleeps you can drop and retake the
> > tty mutex either side of it, so you should be able to do that in the
> > close timeout case. You may need to think about the order of locking with
> > the port mutex but I suspect you can drop and retake both there.
> 
> …basically emulating the BKL semantics? Sounds more doable. I'll look
> into it.

If I understand it correctly, the problem is the msleep_interruptible()
in __uart_wait_until_sent(), right?

Note that this function may be called with or without the port mutex
held, depending on whether the caller is uart_close or uart_wait_until_sent.
The tricky part here will be making sure that you hold neither the
port mutex nor the tty_mutex while sleeping, and to always retake them
in the correct order (tty_mutex before port mutex).

My mistake here must have been that I assumed the timeout was relatively
short to not hurt when holding a mutex, since we already hold the port
mutex. I expected the wait time to be a fraction of a second as in the time
that it takes to send a few remaining characters, which would be acceptable,
unlike the 30 second sleep that you are seeing.

> Of course that means it has to be done individually in all drivers.

Right. Fortunately, we have now reduced the number of drivers a bit, by
moving some of them to staging or completely out of the kernel.

Some drivers call their wait_until_sent function directly from their
close function, some call it through tty_wait_until_sent, and some
actually do both.

Further, some of the drivers have a rather ugly part in them where we take
tty_lock() conditionally in wait_until_sent(), depending on whether the
current thread already holds it (i.e. when coming from ->close, not when
coming from ioctl).

	Arnd

  reply	other threads:[~2011-05-27 11:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-25 23:59 tty_lock held during transmit wait in close: still unresolved Andreas Bombe
2011-05-26  7:11 ` Greg KH
2011-05-27  0:29   ` Andreas Bombe
2011-05-26  8:17 ` Alan Cox
2011-05-27  0:41   ` Andreas Bombe
2011-05-27 11:38     ` Arnd Bergmann [this message]
2011-05-27 12:11     ` Jiri Slaby
2011-05-27 13:53       ` Arnd Bergmann

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=201105271338.54713.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=aeb@debian.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=greg@kroah.com \
    --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.