Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
@ 2020-06-30  3:15 Miao-chen Chou
  2020-06-30  6:51 ` Marcel Holtmann
  2020-07-07 15:38 ` Marcel Holtmann
  0 siblings, 2 replies; 6+ messages in thread
From: Miao-chen Chou @ 2020-06-30  3:15 UTC (permalink / raw)
  To: Bluetooth Kernel Mailing List
  Cc: Marcel Holtmann, Alain Michaud, pavel, Miao-chen Chou,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, linux-kernel, netdev

This fixes the kernel oops by removing unnecessary background scan
update from hci_adv_monitors_clear() which shouldn't invoke any work
queue.

The following test was performed.
- Run "rmmod btusb" and verify that no kernel oops is triggered.

Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 net/bluetooth/hci_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5577cf9e2c7cd..77615161c7d72 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3005,8 +3005,6 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
 		hci_free_adv_monitor(monitor);
 
 	idr_destroy(&hdev->adv_monitors_idr);
-
-	hci_update_background_scan(hdev);
 }
 
 void hci_free_adv_monitor(struct adv_monitor *monitor)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
  2020-06-30  3:15 [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear() Miao-chen Chou
@ 2020-06-30  6:51 ` Marcel Holtmann
  2020-06-30 21:55   ` Miao-chen Chou
  2020-07-07 15:38 ` Marcel Holtmann
  1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2020-06-30  6:51 UTC (permalink / raw)
  To: Miao-chen Chou
  Cc: Bluetooth Kernel Mailing List, Alain Michaud, Pavel Machek,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, linux-kernel, netdev

Hi Miao-chen,

> This fixes the kernel oops by removing unnecessary background scan
> update from hci_adv_monitors_clear() which shouldn't invoke any work
> queue.
> 
> The following test was performed.
> - Run "rmmod btusb" and verify that no kernel oops is triggered.
> 
> Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> net/bluetooth/hci_core.c | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5577cf9e2c7cd..77615161c7d72 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3005,8 +3005,6 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
> 		hci_free_adv_monitor(monitor);
> 
> 	idr_destroy(&hdev->adv_monitors_idr);
> -
> -	hci_update_background_scan(hdev);
> }

I am happy to apply this as well, but I also applied another patch re-arranging the workqueue destroy handling. Can you check which prefer or if we should include both patches.

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
  2020-06-30  6:51 ` Marcel Holtmann
@ 2020-06-30 21:55   ` Miao-chen Chou
  2020-07-06 21:05     ` Miao-chen Chou
  0 siblings, 1 reply; 6+ messages in thread
From: Miao-chen Chou @ 2020-06-30 21:55 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Bluetooth Kernel Mailing List, Alain Michaud, Pavel Machek,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, LKML, netdev

Hi Marcel,

hci_unregister_dev() is invoked when the controller is intended to be
removed by btusb driver. In other words, there should not be any
activity on hdev's workqueue, so the destruction of the workqueue
should be the first thing to do to prevent the clear helpers from
issuing any work. So my suggestion is to revert the patch re-arranging
the workqueue and apply this instead.
I should have uploaded this earlier, but I encountered some troubles
while verifying the changes. Sorry for the inconvenience.

Regards,
Miao

On Mon, Jun 29, 2020 at 11:51 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Miao-chen,
>
> > This fixes the kernel oops by removing unnecessary background scan
> > update from hci_adv_monitors_clear() which shouldn't invoke any work
> > queue.
> >
> > The following test was performed.
> > - Run "rmmod btusb" and verify that no kernel oops is triggered.
> >
> > Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > ---
> >
> > net/bluetooth/hci_core.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 5577cf9e2c7cd..77615161c7d72 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -3005,8 +3005,6 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
> >               hci_free_adv_monitor(monitor);
> >
> >       idr_destroy(&hdev->adv_monitors_idr);
> > -
> > -     hci_update_background_scan(hdev);
> > }
>
> I am happy to apply this as well, but I also applied another patch re-arranging the workqueue destroy handling. Can you check which prefer or if we should include both patches.
>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
  2020-06-30 21:55   ` Miao-chen Chou
@ 2020-07-06 21:05     ` Miao-chen Chou
  0 siblings, 0 replies; 6+ messages in thread
From: Miao-chen Chou @ 2020-07-06 21:05 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Bluetooth Kernel Mailing List, Alain Michaud, Pavel Machek,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, LKML, netdev

Hi Marcel,

In case you missed this thread, my suggestion is to revert the
previous patch and apply this patch. Please see my earlier email for
the reason. Thanks.

Regards,
Miao

On Tue, Jun 30, 2020 at 2:55 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> Hi Marcel,
>
> hci_unregister_dev() is invoked when the controller is intended to be
> removed by btusb driver. In other words, there should not be any
> activity on hdev's workqueue, so the destruction of the workqueue
> should be the first thing to do to prevent the clear helpers from
> issuing any work. So my suggestion is to revert the patch re-arranging
> the workqueue and apply this instead.
> I should have uploaded this earlier, but I encountered some troubles
> while verifying the changes. Sorry for the inconvenience.
>
> Regards,
> Miao
>
> On Mon, Jun 29, 2020 at 11:51 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > Hi Miao-chen,
> >
> > > This fixes the kernel oops by removing unnecessary background scan
> > > update from hci_adv_monitors_clear() which shouldn't invoke any work
> > > queue.
> > >
> > > The following test was performed.
> > > - Run "rmmod btusb" and verify that no kernel oops is triggered.
> > >
> > > Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
> > > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > > ---
> > >
> > > net/bluetooth/hci_core.c | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > > index 5577cf9e2c7cd..77615161c7d72 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -3005,8 +3005,6 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
> > >               hci_free_adv_monitor(monitor);
> > >
> > >       idr_destroy(&hdev->adv_monitors_idr);
> > > -
> > > -     hci_update_background_scan(hdev);
> > > }
> >
> > I am happy to apply this as well, but I also applied another patch re-arranging the workqueue destroy handling. Can you check which prefer or if we should include both patches.
> >
> > Regards
> >
> > Marcel
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
  2020-06-30  3:15 [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear() Miao-chen Chou
  2020-06-30  6:51 ` Marcel Holtmann
@ 2020-07-07 15:38 ` Marcel Holtmann
  2020-07-12  7:46   ` Pavel Machek
  1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2020-07-07 15:38 UTC (permalink / raw)
  To: Miao-chen Chou
  Cc: Bluetooth Kernel Mailing List, Alain Michaud, Pavel Machek,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, linux-kernel, netdev

Hi Miao-chen,

> This fixes the kernel oops by removing unnecessary background scan
> update from hci_adv_monitors_clear() which shouldn't invoke any work
> queue.
> 
> The following test was performed.
> - Run "rmmod btusb" and verify that no kernel oops is triggered.
> 
> Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> net/bluetooth/hci_core.c | 2 --
> 1 file changed, 2 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear()
  2020-07-07 15:38 ` Marcel Holtmann
@ 2020-07-12  7:46   ` Pavel Machek
  0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2020-07-12  7:46 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Miao-chen Chou, Bluetooth Kernel Mailing List, Alain Michaud,
	Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
	Johan Hedberg, linux-kernel, netdev


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

On Tue 2020-07-07 17:38:46, Marcel Holtmann wrote:
> Hi Miao-chen,
> 
> > This fixes the kernel oops by removing unnecessary background scan
> > update from hci_adv_monitors_clear() which shouldn't invoke any work
> > queue.
> > 
> > The following test was performed.
> > - Run "rmmod btusb" and verify that no kernel oops is triggered.
> > 
> > Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > ---
> > 
> > net/bluetooth/hci_core.c | 2 --
> > 1 file changed, 2 deletions(-)
> 
> patch has been applied to bluetooth-next tree.

Bluetooth no longer seems to oops for me... but there's different
showstopper in next (graphics -- i915 -- related). Oh well :-(.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30  3:15 [PATCH v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear() Miao-chen Chou
2020-06-30  6:51 ` Marcel Holtmann
2020-06-30 21:55   ` Miao-chen Chou
2020-07-06 21:05     ` Miao-chen Chou
2020-07-07 15:38 ` Marcel Holtmann
2020-07-12  7:46   ` Pavel Machek

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git