From: Felipe Balbi <balbi@ti.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Alan Cox <alan@linux.intel.com>,
Marcel Holtmann <marcel@holtmann.org>,
Greg KH <gregkh@linuxfoundation.org>,
Muralidharan Karicheri <m-karicheri2@ti.com>,
<linux-bluetooth@vger.kernel.org>, <linux-serial@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: hci_ldsic nested locking problem
Date: Thu, 20 Mar 2014 12:29:20 -0500 [thread overview]
Message-ID: <20140320172920.GC2827@saruman.home> (raw)
In-Reply-To: <20140320171621.GA2827@saruman.home>
[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]
On Thu, Mar 20, 2014 at 12:16:22PM -0500, Felipe Balbi wrote:
> On Thu, Mar 20, 2014 at 04:42:16PM +0000, Alan Cox wrote:
> > On Thu, 2014-03-20 at 11:34 -0500, Felipe Balbi wrote:
> > > Hi,
> > >
> > > when 8250 driver calls uart_write_wakeup(), the tty port lock is already
> > > taken. hci_ldisc.c's implementation of ->write_wakeup() calls
> > > tty->ops->write() to actually send the characters, but that call will
> > > try to acquire the same port lock again.
> > >
> > > Looking at other line disciplines that looks like a bug in hci_ldisc.c.
> > > Am I correct to assume that ->write_wakeup() is supposed to *just*
> > > wakeup the bottom half so we handle ->write() in another context ?
> > >
> > > Is it legal to call tty->ops->write() from within ->write_wakeup() ?
> >
> > It isn't because you might send all the bytes and go
> >
> > write
> > write_wakeup
> > write
> > write wakeup
> > ...
> >
> > and recurse
then we need updates to Documentation:
Documentation/serial/tty.txt::
| Driver Side Interfaces:
|
| receive_buf() - Hand buffers of bytes from the driver to the ldisc
| for processing. Semantics currently rather
| mysterious 8(
|
| write_wakeup() - May be called at any point between open and close.
| The TTY_DO_WRITE_WAKEUP flag indicates if a call
| is needed but always races versus calls. Thus the
| ldisc must be careful about setting order and to
| handle unexpected calls. Must not sleep.
|
| The driver is forbidden from calling this directly
| from the ->write call from the ldisc as the ldisc
| is permitted to call the driver write method from
| this function. In such a situation defer it.
documentation says ldisc is allowed to call ->write() from
->write_wakeup(). huh ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Alan Cox <alan@linux.intel.com>,
Marcel Holtmann <marcel@holtmann.org>,
Greg KH <gregkh@linuxfoundation.org>,
Muralidharan Karicheri <m-karicheri2@ti.com>,
linux-bluetooth@vger.kernel.org, linux-serial@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: hci_ldsic nested locking problem
Date: Thu, 20 Mar 2014 12:29:20 -0500 [thread overview]
Message-ID: <20140320172920.GC2827@saruman.home> (raw)
In-Reply-To: <20140320171621.GA2827@saruman.home>
[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]
On Thu, Mar 20, 2014 at 12:16:22PM -0500, Felipe Balbi wrote:
> On Thu, Mar 20, 2014 at 04:42:16PM +0000, Alan Cox wrote:
> > On Thu, 2014-03-20 at 11:34 -0500, Felipe Balbi wrote:
> > > Hi,
> > >
> > > when 8250 driver calls uart_write_wakeup(), the tty port lock is already
> > > taken. hci_ldisc.c's implementation of ->write_wakeup() calls
> > > tty->ops->write() to actually send the characters, but that call will
> > > try to acquire the same port lock again.
> > >
> > > Looking at other line disciplines that looks like a bug in hci_ldisc.c.
> > > Am I correct to assume that ->write_wakeup() is supposed to *just*
> > > wakeup the bottom half so we handle ->write() in another context ?
> > >
> > > Is it legal to call tty->ops->write() from within ->write_wakeup() ?
> >
> > It isn't because you might send all the bytes and go
> >
> > write
> > write_wakeup
> > write
> > write wakeup
> > ...
> >
> > and recurse
then we need updates to Documentation:
Documentation/serial/tty.txt::
| Driver Side Interfaces:
|
| receive_buf() - Hand buffers of bytes from the driver to the ldisc
| for processing. Semantics currently rather
| mysterious 8(
|
| write_wakeup() - May be called at any point between open and close.
| The TTY_DO_WRITE_WAKEUP flag indicates if a call
| is needed but always races versus calls. Thus the
| ldisc must be careful about setting order and to
| handle unexpected calls. Must not sleep.
|
| The driver is forbidden from calling this directly
| from the ->write call from the ldisc as the ldisc
| is permitted to call the driver write method from
| this function. In such a situation defer it.
documentation says ldisc is allowed to call ->write() from
->write_wakeup(). huh ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2014-03-20 17:29 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-20 16:34 hci_ldsic nested locking problem Felipe Balbi
2014-03-20 16:34 ` Felipe Balbi
2014-03-20 16:42 ` Alan Cox
2014-03-20 16:42 ` Alan Cox
2014-03-20 17:06 ` Kodiak Furr
2014-03-20 17:16 ` Felipe Balbi
2014-03-20 17:16 ` Felipe Balbi
2014-03-20 17:29 ` Felipe Balbi [this message]
2014-03-20 17:29 ` Felipe Balbi
2014-03-20 17:34 ` Peter Hurley
2014-03-20 17:35 ` Felipe Balbi
2014-03-20 17:35 ` Felipe Balbi
2014-03-20 18:45 ` Greg KH
2014-03-20 18:45 ` Greg KH
2014-03-20 18:54 ` Peter Hurley
2014-03-20 17:31 ` Peter Hurley
2014-03-20 18:11 ` Felipe Balbi
2014-03-20 18:11 ` Felipe Balbi
2014-03-20 18:21 ` Peter Hurley
2014-03-20 18:25 ` Felipe Balbi
2014-03-20 18:25 ` Felipe Balbi
2014-03-20 19:01 ` Felipe Balbi
2014-03-20 19:01 ` Felipe Balbi
2014-03-20 19:03 ` Felipe Balbi
2014-03-20 19:03 ` Felipe Balbi
2014-03-20 19:16 ` Peter Hurley
2014-03-20 19:25 ` Felipe Balbi
2014-03-20 19:25 ` Felipe Balbi
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=20140320172920.GC2827@saruman.home \
--to=balbi@ti.com \
--cc=alan@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=m-karicheri2@ti.com \
--cc=marcel@holtmann.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.