From patchwork Tue Jul 31 13:02:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Amit Pundir X-Patchwork-Id: 143185 Delivered-To: patch@linaro.org Received: by 2002:a2e:9754:0:0:0:0:0 with SMTP id f20-v6csp5305662ljj; Tue, 31 Jul 2018 06:02:35 -0700 (PDT) X-Google-Smtp-Source: AAOMgpf0jbABr44uCPSkLwUBCMvbb7wdPi9SSANlf1jcT31wREmeTW4mnySjaQfaBUJ+YvMa/QFK X-Received: by 2002:a65:620b:: with SMTP id d11-v6mr20632471pgv.429.1533042155164; Tue, 31 Jul 2018 06:02:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1533042155; cv=none; d=google.com; s=arc-20160816; b=lu0h83qIncwNVWJCRx2wDp6ZfL+RQwf25HeWNWcSngwHdt47SwEV0zTxVShtNeO3Lj xOEY5lUCh1sz0CxFpKAuKYOTeCeNLHu3eLjG+cJR7TkK5RbnFXR/kWMDviXJUuvCFOmi nLAAfivrXOyQU7WcDx1Bk9SHMSgwj4aIjNFRzOylSOwzH1oPusqdjebOp+V/Jyay70sa Ym10GWDyNk4oryOv4S2L3oJDDGChB+21OkKEn0Owe6RxppEYPTe+/axNhYijdHrXDGGS Q9F0ULxcXwyEsLSI1LbVoe5Z9U8qImKrCoQLep1xKC3hmlx9HdTyZKDOWdjqpY1k/ZU9 mhYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=987NmzXJOVTqgVWJb0hdwtteXyJIlYGzfVaZ2wsqqUM=; b=0FlQTidGTxgQKxGSxCcczZzFTOh2zP+SsEoDwN6V+j+IC4kk6N47KC2MO1FLF+9A+D GoqZxIhWAlqWlGCo3OL1yB0+7Y3JMAFqpRSHHiaHeJPT3mCz2LyDzqaM3Lvw27JIlo9W iU1AVjQrUGwiJzdE0SIQHakcDW9ocEs8wf+6YB6EDqzPH5/3mScdt4RyOWSWlloWd6OE HaC7iB6VAR6AQaFASest5YHHgkGRoYZtHIkv8cVjwaGXsf8WxfNxyygvbG7bdV7YGQYA cl7jDdvTkmwSao1HEKtxJNkKL9NB70886Pidus1ZxCOr/hW37pq8rp3kuhhBkARh/248 BLfw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cz89+h4z; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u35-v6si13052722pgl.237.2018.07.31.06.02.34; Tue, 31 Jul 2018 06:02:35 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cz89+h4z; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732163AbeGaOms (ORCPT + 13 others); Tue, 31 Jul 2018 10:42:48 -0400 Received: from mail-pl0-f67.google.com ([209.85.160.67]:38491 "EHLO mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731962AbeGaOmr (ORCPT ); Tue, 31 Jul 2018 10:42:47 -0400 Received: by mail-pl0-f67.google.com with SMTP id u11-v6so1581745plq.5 for ; Tue, 31 Jul 2018 06:02:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=987NmzXJOVTqgVWJb0hdwtteXyJIlYGzfVaZ2wsqqUM=; b=cz89+h4zi9LiNMHizj85yso2vywqxgSYLgVHqZTYICZRuD/qFHxG+3L+j234QhwfQb GNqsFLTduK+aVTeq4cBJltH5Z34/tpVje8SplnR1Xb/s3pGevzPCugdT3fbZUFLK5CBj /01zxuQNwxv6v/UNtXvAfhVJYw9UJBL9/fFCc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=987NmzXJOVTqgVWJb0hdwtteXyJIlYGzfVaZ2wsqqUM=; b=dTY8pfEC7Jam5azDuyZWG/OkCpQ2ws0gEr4QmXJ/jI88+SkQAbxCBPq/yKXB3HgTrJ aW8Uj76NPSHv3gql75/mKI32rYyvPB/Fc7Bqqs4+OCia5uxwvI9KQ96f4UjmDoszJlVj VXbMJEWHJ1c40RS2qGCDmD8IHe7tey/6EZqXsEvW6FxCuJVSNM8+ukEAjD7b9WOKA4l0 mI1Dw81Hh74Zpol+ZJbMa2jZH1v73sU4pbIz1Kw0W+MrqILfAvT2HRg2k4vj7TSS4WHc o/vaU0qWAawRNhEERPwOZp077glqULScJTS3Nww5Z6gCuS/nn917IkjXeJZLjNI6UyKm 37ug== X-Gm-Message-State: AOUpUlHPLD9OY5ECFfQN79kpEVYeoFS0HqPkk0GhSKXwfxRUa2c0lDau vLGB1vzXI3elEXCld4LgHnmXLK0Q0KY= X-Received: by 2002:a17:902:6105:: with SMTP id t5-v6mr20522265plj.92.1533042152859; Tue, 31 Jul 2018 06:02:32 -0700 (PDT) Received: from localhost.localdomain ([106.51.18.123]) by smtp.gmail.com with ESMTPSA id f23-v6sm23048473pgl.5.2018.07.31.06.02.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 31 Jul 2018 06:02:31 -0700 (PDT) From: Amit Pundir To: Greg KH , Stable Cc: =?utf-8?q?Ronald_Tschal=C3=A4r?= , Marcel Holtmann Subject: [PATCH for-4.14.y 3/5] Bluetooth: hci_ldisc: Allow sleeping while proto locks are held. Date: Tue, 31 Jul 2018 18:32:19 +0530 Message-Id: <1533042142-8681-3-git-send-email-amit.pundir@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1533042142-8681-1-git-send-email-amit.pundir@linaro.org> References: <1533042142-8681-1-git-send-email-amit.pundir@linaro.org> MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Ronald Tschalär commit 67d2f8781b9f00d1089aafcfa3dc09fcd0f343e2 upstream. Commit dec2c92880cc5435381d50e3045ef018a762a917 ("Bluetooth: hci_ldisc: Use rwlocking to avoid closing proto races") introduced locks in hci_ldisc that are held while calling the proto functions. These locks are rwlock's, and hence do not allow sleeping while they are held. However, the proto functions that hci_bcm registers use mutexes and hence need to be able to sleep. In more detail: hci_uart_tty_receive() and hci_uart_dequeue() both acquire the rwlock, after which they call proto->recv() and proto->dequeue(), respectively. In the case of hci_bcm these point to bcm_recv() and bcm_dequeue(). The latter both acquire the bcm_device_lock, which is a mutex, so doing so results in a call to might_sleep(). But since we're holding a rwlock in hci_ldisc, that results in the following BUG (this for the dequeue case - a similar one for the receive case is omitted for brevity): BUG: sleeping function called from invalid context at kernel/locking/mutex.c in_atomic(): 1, irqs_disabled(): 0, pid: 7303, name: kworker/7:3 INFO: lockdep is turned off. CPU: 7 PID: 7303 Comm: kworker/7:3 Tainted: G W OE 4.13.2+ #17 Hardware name: Apple Inc. MacBookPro13,3/Mac-A5C67F76ED83108C, BIOS MBP133.8 Workqueue: events hci_uart_write_work [hci_uart] Call Trace: dump_stack+0x8e/0xd6 ___might_sleep+0x164/0x250 __might_sleep+0x4a/0x80 __mutex_lock+0x59/0xa00 ? lock_acquire+0xa3/0x1f0 ? lock_acquire+0xa3/0x1f0 ? hci_uart_write_work+0xd3/0x160 [hci_uart] mutex_lock_nested+0x1b/0x20 ? mutex_lock_nested+0x1b/0x20 bcm_dequeue+0x21/0xc0 [hci_uart] hci_uart_write_work+0xe6/0x160 [hci_uart] process_one_work+0x253/0x6a0 worker_thread+0x4d/0x3b0 kthread+0x133/0x150 We can't replace the mutex in hci_bcm, because there are other calls there that might sleep. Therefore this replaces the rwlock's in hci_ldisc with rw_semaphore's (which allow sleeping). This is a safer approach anyway as it reduces the restrictions on the proto callbacks. Also, because acquiring write-lock is very rare compared to acquiring the read-lock, the percpu variant of rw_semaphore is used. Lastly, because hci_uart_tx_wakeup() may be called from an IRQ context, we can't block (sleep) while trying acquire the read lock there, so we use the trylock variant. Signed-off-by: Ronald Tschalär Reviewed-by: Lukas Wunner Signed-off-by: Marcel Holtmann Signed-off-by: Amit Pundir --- Not applicable for 4.9.y, 4.4.y and 3.18.y drivers/bluetooth/hci_ldisc.c | 38 ++++++++++++++++++++++---------------- drivers/bluetooth/hci_uart.h | 2 +- 2 files changed, 23 insertions(+), 17 deletions(-) -- 2.7.4 diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 6aef3bde10d7..c823914b3a80 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -115,12 +115,12 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu) struct sk_buff *skb = hu->tx_skb; if (!skb) { - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) skb = hu->proto->dequeue(hu); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); } else { hu->tx_skb = NULL; } @@ -130,7 +130,14 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu) int hci_uart_tx_wakeup(struct hci_uart *hu) { - read_lock(&hu->proto_lock); + /* This may be called in an IRQ context, so we can't sleep. Therefore + * we try to acquire the lock only, and if that fails we assume the + * tty is being closed because that is the only time the write lock is + * acquired. If, however, at some point in the future the write lock + * is also acquired in other situations, then this must be revisited. + */ + if (!percpu_down_read_trylock(&hu->proto_lock)) + return 0; if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) goto no_schedule; @@ -145,7 +152,7 @@ int hci_uart_tx_wakeup(struct hci_uart *hu) schedule_work(&hu->write_work); no_schedule: - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return 0; } @@ -247,12 +254,12 @@ static int hci_uart_flush(struct hci_dev *hdev) tty_ldisc_flush(tty); tty_driver_flush_buffer(tty); - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) hu->proto->flush(hu); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return 0; } @@ -275,15 +282,15 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb) BT_DBG("%s: type %d len %d", hdev->name, hci_skb_pkt_type(skb), skb->len); - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return -EUNATCH; } hu->proto->enqueue(hu, skb); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); hci_uart_tx_wakeup(hu); @@ -486,7 +493,7 @@ static int hci_uart_tty_open(struct tty_struct *tty) INIT_WORK(&hu->init_ready, hci_uart_init_work); INIT_WORK(&hu->write_work, hci_uart_write_work); - rwlock_init(&hu->proto_lock); + percpu_init_rwsem(&hu->proto_lock); /* Flush any pending characters in the driver */ tty_driver_flush_buffer(tty); @@ -503,7 +510,6 @@ static void hci_uart_tty_close(struct tty_struct *tty) { struct hci_uart *hu = tty->disc_data; struct hci_dev *hdev; - unsigned long flags; BT_DBG("tty %p", tty); @@ -518,9 +524,9 @@ static void hci_uart_tty_close(struct tty_struct *tty) hci_uart_close(hdev); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - write_lock_irqsave(&hu->proto_lock, flags); + percpu_down_write(&hu->proto_lock); clear_bit(HCI_UART_PROTO_READY, &hu->flags); - write_unlock_irqrestore(&hu->proto_lock, flags); + percpu_up_write(&hu->proto_lock); cancel_work_sync(&hu->write_work); @@ -582,10 +588,10 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, if (!hu || tty != hu->tty) return; - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return; } @@ -593,7 +599,7 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, * tty caller */ hu->proto->recv(hu, data, count); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); if (hu->hdev) hu->hdev->stat.byte_rx += count; diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h index d9cd95d81149..66e8c68e4607 100644 --- a/drivers/bluetooth/hci_uart.h +++ b/drivers/bluetooth/hci_uart.h @@ -87,7 +87,7 @@ struct hci_uart { struct work_struct write_work; const struct hci_uart_proto *proto; - rwlock_t proto_lock; /* Stop work for proto close */ + struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */ void *priv; struct sk_buff *tx_skb;