From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752728AbcBVAIa (ORCPT ); Sun, 21 Feb 2016 19:08:30 -0500 Received: from mailout3.w1.samsung.com ([210.118.77.13]:27644 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752537AbcBVAI2 (ORCPT ); Sun, 21 Feb 2016 19:08:28 -0500 X-AuditID: cbfec7f4-f79026d00000418a-c4-56ca5179a650 Subject: Re: [PATCHv2] serial: samsung: drop the spinlock around uart_write_wakeup To: Anand Moon References: <1455858937-2380-1-git-send-email-linux.amoon@gmail.com> <56C76D3D.3010009@hurleysoftware.com> Cc: Peter Hurley , Greg Kroah-Hartman , Jiri Slaby , linux-serial@vger.kernel.org, "linux-samsung-soc@vger.kernel.org" , Linux Kernel , Marek Szyprowski , =?UTF-8?Q?Robert_Ba=c5=82dyga?= From: Krzysztof Kozlowski Message-id: <56CA5174.3050706@samsung.com> Date: Mon, 22 Feb 2016 09:08:20 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDLMWRmVeSWpSXmKPExsVy+t/xa7qVgafCDB4vlLRoXryezWLKhg9M Fq9fGFpc3jWHzWLG+X1MFmcW97JbrNt4i91i7ZG77BbHLv1jtHhweCe7A5fHzll32T1W/FnP 5rF/7hp2j74tqxg91m+5yuLxeZNcAFsUl01Kak5mWWqRvl0CV0b7UYuCdSIVbZ/OszYwbhXo YuTkkBAwkZh+5ykrhC0mceHeerYuRi4OIYGljBLrFkxiAkkICTxllJi3KQzEFhYIkXiyeysb iC0ioCZx5ekKVoiGX4wS/ctbmUAcZoEGZonTZ+8xg1SxCRhLbF6+BKyDV0BL4srJXrCpLAKq Ei9bnoCtFhWIkDjc2cUOUSMo8WPyPZYuRg4OToFgiUtP9UFMZgF1iSlTckEqmAXkJTavecs8 gVFgFpKGWQhVs5BULWBkXsUomlqaXFCclJ5rqFecmFtcmpeul5yfu4kREgdfdjAuPmZ1iFGA g1GJh7eB5VSYEGtiWXFl7iFGCQ5mJRHetosnw4R4UxIrq1KL8uOLSnNSiw8xSnOwKInzzt31 PkRIID2xJDU7NbUgtQgmy8TBKdXAyPDt6xaet+o/c0XkUssmf273+zJn3Sflx4wREmre2QnX L364zCyvm2WY8Mr39HSJeznPYh6wrDz2U8N3Z8v1T3ynT2l1vosw0NucvfdUlemRra+l5Iof fzFcxy8+zXlDYKbYuWetN2Pe5TC5HNq1jIU9wOZ10XKNemMx/Y+du7z5vWZLnG7+ocRSnJFo qMVcVJwIAITWGFp/AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21.02.2016 11:59, Anand Moon wrote: > Hi Krzysztof, > > On 21 February 2016 at 07:07, Krzysztof Kozlowski > wrote: >> 2016-02-20 4:30 GMT+09:00 Peter Hurley : >>> [ +cc Krzysztof Kozlowski ] >>> >>> On 02/18/2016 09:15 PM, Anand Moon wrote: >>>> From: Anand Moon >>>> >>>> drop the spin_unlock/lock around uart_write_wakeup to protect >>>> write wakeup for uart port. >>> >>> What Krzysztof was saying wrt v1 of this patch is that the >>> changelog should provide as much information as possible to >>> the maintainer(s) and driver author(s), and that you should >>> test that outcome. >>> >>> Here's what I would have written for a commit message: >>> >>> >>> Remove deadlock workaround for line disciplines that invoke >>> the tty driver's write() method directly from their write_wakeup() >>> method. As documented for the write_wakeup() line discipline method >>> in tty_ldisc.h, line disciplines must not attempt i/o directly >>> from write_wakeup() as this will deadlock. Reviews of in-tree line >>> disciplines confirm all defer i/o. >>> >>> NB: This workaround was added in commit c15c3747ee32 >>> ("serial: samsung: fix potential soft lockup during uart write") >>> which notes both slip and bluetooth hci attempt i/o directly from >>> write_wakeup(). These issues were fixed in commits 661f7fda21b1 >>> ("slip: Fix deadlock in write_wakeup") and da64c27d3c93 >>> ("bluetooth: hci_ldisc: fix deadlock condition"), respectively. >> >> Thanks Peter for thorough analysis. It shouldn't be done by you but by >> the patch submitter... and I have big worries that Anand did not >> perform that analysis. >> >> Anand, could you at least test that this lockup does not happen >> anymore? You will need board with Bluetooth for that (and not USB >> Bluetooth...). If you cannot test it, maybe guys from Polish R&D could >> help you (Cc-ed), because they were working on DMA for serial used in >> Bluetooth. >> >> Best regards, >> Krzysztof > > I have looked into the history of the changes, > commit c15c3747ee32 (serial: samsung: fix potential soft lockup during > uart write) > was added long time ago, that's why have missed this. > > I don't have on-board Bluetooth enable boards with me, > so if their is potential lockup you people observer > do not consider this patch. Which means that you cannot test it... Find someone who can test it with Bluetooth (or SLIP) and will provide you a Tested-by tag. Best regards, Krzysztof