| Message ID | 1395343807-21618-10-git-send-email-balbi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Felipe Balbi <balbi@ti.com> [140320 12:39]: > This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. > > That commit tried to fix a deadlock problem when using > hci_ldisc, but it turns out the bug was in hci_ldsic > all along where it was calling ->write() from within > ->write_wakeup() callback. > > The problem is that ->write_wakeup() was called with > port lock held and ->write() tried to grab the same > port lock. Should this and the next patch be earlier in the series as a fix for the v3.15-rc cycle? Should they be cc: stable as well? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/25/2014 02:28 PM, Tony Lindgren wrote: > * Felipe Balbi <balbi@ti.com> [140320 12:39]: >> This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. >> >> That commit tried to fix a deadlock problem when using >> hci_ldisc, but it turns out the bug was in hci_ldsic >> all along where it was calling ->write() from within >> ->write_wakeup() callback. >> >> The problem is that ->write_wakeup() was called with >> port lock held and ->write() tried to grab the same >> port lock. > > Should this and the next patch be earlier in the series > as a fix for the v3.15-rc cycle? Should they be cc: stable > as well? Well, right now the other fix has had _zero_ testing so not really a -stable candidate just yet. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote: > On 03/25/2014 02:28 PM, Tony Lindgren wrote: > >* Felipe Balbi <balbi@ti.com> [140320 12:39]: > >>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. > >> > >>That commit tried to fix a deadlock problem when using > >>hci_ldisc, but it turns out the bug was in hci_ldsic > >>all along where it was calling ->write() from within > >>->write_wakeup() callback. > >> > >>The problem is that ->write_wakeup() was called with > >>port lock held and ->write() tried to grab the same > >>port lock. > > > >Should this and the next patch be earlier in the series > >as a fix for the v3.15-rc cycle? Should they be cc: stable > >as well? > > Well, right now the other fix has had _zero_ testing > so not really a -stable candidate just yet. how can you even say that ? Unless you work for some 3 letter acronym organizations, you have no clue about the fact that this was tested on a keystone 2 platform. How else would we have found the issue to start with ?
On 03/26/2014 10:10 PM, Felipe Balbi wrote: > Hi, > > On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote: >> On 03/25/2014 02:28 PM, Tony Lindgren wrote: >>> * Felipe Balbi <balbi@ti.com> [140320 12:39]: >>>> This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. >>>> >>>> That commit tried to fix a deadlock problem when using >>>> hci_ldisc, but it turns out the bug was in hci_ldsic >>>> all along where it was calling ->write() from within >>>> ->write_wakeup() callback. >>>> >>>> The problem is that ->write_wakeup() was called with >>>> port lock held and ->write() tried to grab the same >>>> port lock. >>> >>> Should this and the next patch be earlier in the series >>> as a fix for the v3.15-rc cycle? Should they be cc: stable >>> as well? >> >> Well, right now the other fix has had _zero_ testing >> so not really a -stable candidate just yet. > > how can you even say that ? I misunderstood when you wrote: On 03/20/2014 02:11 PM, Felipe Balbi wrote: > here's a build-tested only patch which is waiting for testing from other > colleagues who've got a platform to reproduce the problem: and then the version I reviewed had no Tested-by: tags. > Unless you work for some 3 letter acronym > organizations, you have no clue about the fact that this was tested on a > keystone 2 platform. Ok. > How else would we have found the issue to start with ? Bug report? Regards, Peter Hurley -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Mar 26, 2014 at 10:27:13PM -0400, Peter Hurley wrote: > On 03/26/2014 10:10 PM, Felipe Balbi wrote: > >Hi, > > > >On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote: > >>On 03/25/2014 02:28 PM, Tony Lindgren wrote: > >>>* Felipe Balbi <balbi@ti.com> [140320 12:39]: > >>>>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. > >>>> > >>>>That commit tried to fix a deadlock problem when using > >>>>hci_ldisc, but it turns out the bug was in hci_ldsic > >>>>all along where it was calling ->write() from within > >>>>->write_wakeup() callback. > >>>> > >>>>The problem is that ->write_wakeup() was called with > >>>>port lock held and ->write() tried to grab the same > >>>>port lock. > >>> > >>>Should this and the next patch be earlier in the series > >>>as a fix for the v3.15-rc cycle? Should they be cc: stable > >>>as well? > >> > >>Well, right now the other fix has had _zero_ testing > >>so not really a -stable candidate just yet. > > > >how can you even say that ? > > I misunderstood when you wrote: > > On 03/20/2014 02:11 PM, Felipe Balbi wrote: > > here's a build-tested only patch which is waiting for testing from other > > colleagues who've got a platform to reproduce the problem: > > and then the version I reviewed had no Tested-by: tags. I wouldn't add that tag myself, but Murali (in Cc) did help testing together with other colleagues. > >How else would we have found the issue to start with ? > > Bug report? touchè :-)
>-----Original Message----- >From: Balbi, Felipe >Sent: Wednesday, March 26, 2014 11:37 PM >To: Peter Hurley >Cc: Balbi, Felipe; Tony Lindgren; Greg KH; linux-serial@vger.kernel.org; linux- >bluetooth@vger.kernel.org; Karicheri, Muralidharan; b32955@freescale.com; Linux OMAP >Mailing List; Linux Kernel Mailing List >Subject: Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock" > >Hi, > >On Wed, Mar 26, 2014 at 10:27:13PM -0400, Peter Hurley wrote: >> On 03/26/2014 10:10 PM, Felipe Balbi wrote: >> >Hi, >> > >> >On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote: >> >>On 03/25/2014 02:28 PM, Tony Lindgren wrote: >> >>>* Felipe Balbi <balbi@ti.com> [140320 12:39]: >> >>>>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. >> >>>> >> >>>>That commit tried to fix a deadlock problem when using hci_ldisc, >> >>>>but it turns out the bug was in hci_ldsic all along where it was >> >>>>calling ->write() from within >> >>>>->write_wakeup() callback. >> >>>> >> >>>>The problem is that ->write_wakeup() was called with port lock >> >>>>held and ->write() tried to grab the same port lock. >> >>> >> >>>Should this and the next patch be earlier in the series as a fix >> >>>for the v3.15-rc cycle? Should they be cc: stable as well? >> >> >> >>Well, right now the other fix has had _zero_ testing so not really a >> >>-stable candidate just yet. >> > >> >how can you even say that ? >> >> I misunderstood when you wrote: >> >> On 03/20/2014 02:11 PM, Felipe Balbi wrote: >> > here's a build-tested only patch which is waiting for testing from >> > other colleagues who've got a platform to reproduce the problem: >> >> and then the version I reviewed had no Tested-by: tags. > >I wouldn't add that tag myself, but Murali (in Cc) did help testing together with other Yes. One of our customer did the test as the problem was reported by the customer. The deadlock fix patch from Balbi (hci_ldsic) fixed the issue and customer confirmed that the issue is fixed. Murali >colleagues. > >> >How else would we have found the issue to start with ? >> >> Bug report? > >touchè :-) > >-- >balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index b092d3d..be10e7e 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -380,11 +380,8 @@ static void transmit_chars(struct uart_omap_port *up, unsigned int lsr) break; } while (--count > 0); - if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) { - spin_unlock(&up->port.lock); + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) uart_write_wakeup(&up->port); - spin_lock(&up->port.lock); - } if (uart_circ_empty(xmit)) serial_omap_stop_tx(&up->port);
This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e. That commit tried to fix a deadlock problem when using hci_ldisc, but it turns out the bug was in hci_ldsic all along where it was calling ->write() from within ->write_wakeup() callback. The problem is that ->write_wakeup() was called with port lock held and ->write() tried to grab the same port lock. Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/tty/serial/omap-serial.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)