All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@ti.com>
To: Alan Cox <alan@linux.intel.com>
Cc: <balbi@ti.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:16:22 -0500	[thread overview]
Message-ID: <20140320171621.GA2827@saruman.home> (raw)
In-Reply-To: <1395333736.22077.32.camel@acox1-desk.ger.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

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

cool, so there really is a bug in hci_ldisc. Marcel, any tips on how do
you want this to be sorted out ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
To: Alan Cox <alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: balbi-l0cyMroinI0@public.gmane.org,
	Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>,
	Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Muralidharan Karicheri
	<m-karicheri2-l0cyMroinI0@public.gmane.org>,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: hci_ldsic nested locking problem
Date: Thu, 20 Mar 2014 12:16:22 -0500	[thread overview]
Message-ID: <20140320171621.GA2827@saruman.home> (raw)
In-Reply-To: <1395333736.22077.32.camel-wU3TRTJX3O1FGiH78xh5akvbDziVy8sZEvhb3Hwu1Ks@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

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

cool, so there really is a bug in hci_ldisc. Marcel, any tips on how do
you want this to be sorted out ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2014-03-20 17:16 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 [this message]
2014-03-20 17:16     ` Felipe Balbi
2014-03-20 17:29     ` Felipe Balbi
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=20140320171621.GA2827@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.