From patchwork Thu Apr 19 15:29:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Keeping X-Patchwork-Id: 901335 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=metanate.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=metanate.com header.i=@metanate.com header.b="B9EzwPEz"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40Rk1p2t5Wz9s19 for ; Fri, 20 Apr 2018 01:49:18 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753431AbeDSPq5 (ORCPT ); Thu, 19 Apr 2018 11:46:57 -0400 Received: from dougal.metanate.com ([90.155.101.14]:42020 "EHLO metanate.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752634AbeDSPqz (ORCPT ); Thu, 19 Apr 2018 11:46:55 -0400 X-Greylist: delayed 1020 seconds by postgrey-1.27 at vger.kernel.org; Thu, 19 Apr 2018 11:46:55 EDT DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=simple/simple; d=metanate.com; s=stronger; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=YD/oLeMfF0OCHFdqVFJuOMbQg4qx444EhCgdgxIr3sM=; b=B9EzwPEzfR0OtBGukljjJ/rkCh q5Z71JL0CheGioLzp+D1w7ZR181z17mPQ8A0qo4j168Qj2H4y7jH6HOmsYZOK/bSCZFH5PtjbozyT c5NTnuHk1wZsI9nzMViofyeIX4jIAxIuBH5HlgXPzJXO885G+AfcAV54uJxUdI2u5lzaQyIWP7dJl ndoVmVpWxqb9yv9CFfmuo0N/YxII7pDgyN7M67fk4DfuGwxnBQEpqnqkJlvUcCtZ2USUUQGCHcXdK 6WYHEMZqIn1zlAFS6fMzLu4rMDc9+HFMxTVMZcbrLWIxOmrqTUrg5yXUF0oSWIFCcqI7QP2CAVzUB NxKiFeYw==; Received: from 188-39-28-98.static.enta.net ([188.39.28.98] helo=localhost.localdomain) by shrek.metanate.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.90_1) (envelope-from ) id 1f9BVI-0005S9-Tm; Thu, 19 Apr 2018 16:29:45 +0100 From: John Keeping To: linux-bluetooth@vger.kernel.org Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Marcel Holtmann , Johan Hedberg , John Keeping Subject: [PATCH] Bluetooth: use wait_event API instead of open-coding it Date: Thu, 19 Apr 2018 16:29:37 +0100 Message-Id: <20180419152937.13515-1-john@metanate.com> X-Mailer: git-send-email 2.17.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org I've seen timeout errors from HCI commands where it looks like schedule_timeout() has returned immediately; additional logging for the error case gives: req_status=1 req_result=0 remaining=10000 jiffies so the device is still in state HCI_REQ_PEND and the value returned by schedule_timeout() is the same as the original timeout (HCI_INIT_TIMEOUT on a system with HZ=1000). Use wait_event_interruptible_timeout() instead of open-coding similar behaviour which is subject to the spurious failure described above. Signed-off-by: John Keeping --- I saw problems with the -rt patchset on 4.9 and I'm not convinced that it's Bluetooth at fault for the problem described above, but I think this is a nice cleanup even if it's not a bug fix. net/bluetooth/hci_request.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c index 66c0781773df..e44d34734834 100644 --- a/net/bluetooth/hci_request.c +++ b/net/bluetooth/hci_request.c @@ -122,7 +122,6 @@ void hci_req_sync_cancel(struct hci_dev *hdev, int err) struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, const void *param, u8 event, u32 timeout) { - DECLARE_WAITQUEUE(wait, current); struct hci_request req; struct sk_buff *skb; int err = 0; @@ -135,21 +134,14 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, hdev->req_status = HCI_REQ_PEND; - add_wait_queue(&hdev->req_wait_q, &wait); - set_current_state(TASK_INTERRUPTIBLE); - err = hci_req_run_skb(&req, hci_req_sync_complete); - if (err < 0) { - remove_wait_queue(&hdev->req_wait_q, &wait); - set_current_state(TASK_RUNNING); + if (err < 0) return ERR_PTR(err); - } - schedule_timeout(timeout); + err = wait_event_interruptible_timeout(hdev->req_wait_q, + hdev->req_status != HCI_REQ_PEND, timeout); - remove_wait_queue(&hdev->req_wait_q, &wait); - - if (signal_pending(current)) + if (err == -ERESTARTSYS) return ERR_PTR(-EINTR); switch (hdev->req_status) { @@ -197,7 +189,6 @@ int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req, unsigned long opt, u32 timeout, u8 *hci_status) { struct hci_request req; - DECLARE_WAITQUEUE(wait, current); int err = 0; BT_DBG("%s start", hdev->name); @@ -213,16 +204,10 @@ int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req, return err; } - add_wait_queue(&hdev->req_wait_q, &wait); - set_current_state(TASK_INTERRUPTIBLE); - err = hci_req_run_skb(&req, hci_req_sync_complete); if (err < 0) { hdev->req_status = 0; - remove_wait_queue(&hdev->req_wait_q, &wait); - set_current_state(TASK_RUNNING); - /* ENODATA means the HCI request command queue is empty. * This can happen when a request with conditionals doesn't * trigger any commands to be sent. This is normal behavior @@ -240,11 +225,10 @@ int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req, return err; } - schedule_timeout(timeout); - - remove_wait_queue(&hdev->req_wait_q, &wait); + err = wait_event_interruptible_timeout(hdev->req_wait_q, + hdev->req_status != HCI_REQ_PEND, timeout); - if (signal_pending(current)) + if (err == -ERESTARTSYS) return -EINTR; switch (hdev->req_status) {