| Message ID | 20200629201441.v1.1.I162e3c6c4f4d963250c37733c3428329110c5989@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1] Bluetooth: Fix kernel oops triggered by hci_adv_monitors_clear() | expand |
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
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 >
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 > >
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
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
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)