From: Miao-chen Chou <[email protected]>
This calls hci_update_background_scan() when there is any update on the
advertisement monitors. If there is at least one advertisement monitor,
the filtering policy of scan parameters should be 0x00. This also reports
device found mgmt events if there is at least one monitor.
The following cases were tested with btmgmt advmon-* commands.
(1) add a ADV monitor and observe that the passive scanning is
triggered.
(2) remove the last ADV monitor and observe that the passive scanning is
terminated.
(3) with a LE peripheral paired, repeat (1) and observe the passive
scanning continues.
(4) with a LE peripheral paired, repeat (2) and observe the passive
scanning continues.
(5) with a ADV monitor, suspend/resume the host and observe the passive
scanning continues.
Signed-off-by: Miao-chen Chou <[email protected]>
Signed-off-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 13 +++++++++++++
net/bluetooth/hci_event.c | 5 +++--
net/bluetooth/hci_request.c | 17 ++++++++++++++---
net/bluetooth/mgmt.c | 5 ++++-
5 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c54f9295892e..524057598ffd 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1284,6 +1284,7 @@ void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_is_adv_monitoring(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 59132b3e2cde..7959b851cc63 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3005,6 +3005,8 @@ 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)
@@ -3038,6 +3040,9 @@ int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
hdev->adv_monitors_cnt++;
monitor->handle = handle;
+
+ hci_update_background_scan(hdev);
+
return 0;
}
@@ -3069,9 +3074,17 @@ int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle)
idr_for_each(&hdev->adv_monitors_idr, &free_adv_monitor, hdev);
}
+ hci_update_background_scan(hdev);
+
return 0;
}
+/* This function requires the caller holds hdev->lock */
+bool hci_is_adv_monitoring(struct hci_dev *hdev)
+{
+ return !idr_is_empty(&hdev->adv_monitors_idr);
+}
+
struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *bdaddr_list,
bdaddr_t *bdaddr, u8 type)
{
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8981954ff4c4..e08d4dd9a24e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5447,14 +5447,15 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
/* Passive scanning shouldn't trigger any device found events,
* except for devices marked as CONN_REPORT for which we do send
- * device found events.
+ * device found events, or advertisement monitoring requested.
*/
if (hdev->le_scan_type == LE_SCAN_PASSIVE) {
if (type == LE_ADV_DIRECT_IND)
return;
if (!hci_pend_le_action_lookup(&hdev->pend_le_reports,
- bdaddr, bdaddr_type))
+ bdaddr, bdaddr_type) &&
+ idr_is_empty(&hdev->adv_monitors_idr))
return;
if (type == LE_ADV_NONCONN_IND || type == LE_ADV_SCAN_IND)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index eee9c007a5fb..29decd7e8051 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -413,11 +413,15 @@ static void __hci_update_background_scan(struct hci_request *req)
*/
hci_discovery_filter_clear(hdev);
+ BT_DBG("%s ADV monitoring is %s", hdev->name,
+ hci_is_adv_monitoring(hdev) ? "on" : "off");
+
if (list_empty(&hdev->pend_le_conns) &&
- list_empty(&hdev->pend_le_reports)) {
+ list_empty(&hdev->pend_le_reports) &&
+ !hci_is_adv_monitoring(hdev)) {
/* If there is no pending LE connections or devices
- * to be scanned for, we should stop the background
- * scanning.
+ * to be scanned for or no ADV monitors, we should stop the
+ * background scanning.
*/
/* If controller is not scanning we are done. */
@@ -794,6 +798,13 @@ static u8 update_white_list(struct hci_request *req)
return 0x00;
}
+ /* Once the controller offloading of advertisement monitor is in place,
+ * the if condition should include the support of MSFT extension
+ * support.
+ */
+ if (!idr_is_empty(&hdev->adv_monitors_idr))
+ return 0x00;
+
/* Select filter policy to use white list */
return 0x01;
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b194da4de2d7..ec66160a673c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -8575,8 +8575,11 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
if (!hci_discovery_active(hdev)) {
if (link_type == ACL_LINK)
return;
- if (link_type == LE_LINK && list_empty(&hdev->pend_le_reports))
+ if (link_type == LE_LINK &&
+ list_empty(&hdev->pend_le_reports) &&
+ !hci_is_adv_monitoring(hdev)) {
return;
+ }
}
if (hdev->discovery.result_filtering) {
--
2.26.2
Hi
On 17.06.2020 16:39, Marcel Holtmann wrote:
> From: Miao-chen Chou <[email protected]>
>
> This calls hci_update_background_scan() when there is any update on the
> advertisement monitors. If there is at least one advertisement monitor,
> the filtering policy of scan parameters should be 0x00. This also reports
> device found mgmt events if there is at least one monitor.
>
> The following cases were tested with btmgmt advmon-* commands.
> (1) add a ADV monitor and observe that the passive scanning is
> triggered.
> (2) remove the last ADV monitor and observe that the passive scanning is
> terminated.
> (3) with a LE peripheral paired, repeat (1) and observe the passive
> scanning continues.
> (4) with a LE peripheral paired, repeat (2) and observe the passive
> scanning continues.
> (5) with a ADV monitor, suspend/resume the host and observe the passive
> scanning continues.
>
> Signed-off-by: Miao-chen Chou <[email protected]>
> Signed-off-by: Marcel Holtmann <[email protected]>
This patch landed recently in linux-next as commit 8208f5a9d435
("Bluetooth: Update background scan and report device based on
advertisement monitors").
It causes a regression, a kernel oops during system suspend/resume cycle
on Samsung Exynos5250 based Snow Chromebook:
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = 86c149f5
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in: cmac cros_ec_sysfs cros_ec_lightbar cros_ec_debugfs
cros_ec_chardev cros_ec_keyb cros_ec_dev snd_soc_hdmi_codec cros_ec_i2c
cros_ec snd_soc_snow snd_soc_i2s snd_soc_idma snd_soc_s3c_dma exynosdrm
analogix_dp exynos_gsc v4l2_mem2mem snd_soc_max98095 snd_soc_core
ac97_bus snd_pcm_dmaengine snd_pcm snd_timer nxp_ptn3460 snd soundcore
pwm_samsung spi_s3c64xx cyapatp crc_itu_t mwifiex_sdio mwifiex
sha256_generic libsha256 sha256_arm btmrvl_sdio btmrvl cfg80211
bluetooth s5p_mfc governor_simpleondemand videobuf2_dma_contig
videobuf2_memops videobuf2_v4l2 ecdh_generic ecc videobuf2_common
videodev phy_exynos_usb2 ohci_exynos panfrost gpu_sched mc s3c2410_wdt
s5p_sss s5p_cec exynos_rng rtc_s3c i2c_arb_gpio_challenge
CPU: 1 PID: 16 Comm: kworker/1:0 Not tainted
5.7.0-rc7-02995-g8208f5a9d435 #8564
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_freezable mmc_rescan
PC is at __queue_work+0x6c/0x4e8
LR is at __queue_work+0x68/0x4e8
pc : [<c03619d8>] lr : [<c03619d4>] psr: 60000093
...
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 6be2406a DAC: 00000051
Process kworker/1:0 (pid: 16, stack limit = 0xf9898f75)
Stack: (0xee117de0 to 0xee118000)
...
[<c03619d8>] (__queue_work) from [<c0361e94>] (queue_work_on+0x40/0x4c)
[<c0361e94>] (queue_work_on) from [<bf17dd0c>]
(hci_adv_monitors_clear+0x74/0x90 [bluetooth])
[<bf17dd0c>] (hci_adv_monitors_clear [bluetooth]) from [<bf17e138>]
(hci_unregister_dev+0x158/0x224 [bluetooth])
[<bf17e138>] (hci_unregister_dev [bluetooth]) from [<bf1d1638>]
(btmrvl_remove_card+0x58/0x7c [btmrvl])
[<bf1d1638>] (btmrvl_remove_card [btmrvl]) from [<c0d09698>]
(sdio_bus_remove+0x30/0x11c)
[<c0d09698>] (sdio_bus_remove) from [<c09d534c>]
(device_release_driver_internal+0xe8/0x1ac)
[<c09d534c>] (device_release_driver_internal) from [<c09d3e18>]
(bus_remove_device+0xcc/0xf8)
[<c09d3e18>] (bus_remove_device) from [<c09cfa3c>] (device_del+0x15c/0x384)
[<c09cfa3c>] (device_del) from [<c0d098b8>] (sdio_remove_func+0x20/0x34)
[<c0d098b8>] (sdio_remove_func) from [<c0d075bc>]
(mmc_sdio_remove+0x38/0x64)
[<c0d075bc>] (mmc_sdio_remove) from [<c0d08858>] (mmc_sdio_detect+0x6c/0xf8)
[<c0d08858>] (mmc_sdio_detect) from [<c0cff6f0>] (mmc_rescan+0x1d0/0x42c)
[<c0cff6f0>] (mmc_rescan) from [<c0362454>] (process_one_work+0x178/0x4ac)
[<c0362454>] (process_one_work) from [<c0362b44>] (worker_thread+0x2c/0x530)
[<c0362b44>] (worker_thread) from [<c0368610>] (kthread+0x12c/0x158)
[<c0368610>] (kthread) from [<c03001a8>] (ret_from_fork+0x14/0x2c)
Exception stack(0xee117fb0 to 0xee117ff8)
...
---[ end trace 0ec00d142e0a49cf ]---
This board uses btmrvl_sdio bluetooth driver if that helps. Reverting
this commit in linux-next 20200629 'fixes' the issue.
I can do more tests if needed on this hardware, just let me know how can
I help and what to do.
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 13 +++++++++++++
> net/bluetooth/hci_event.c | 5 +++--
> net/bluetooth/hci_request.c | 17 ++++++++++++++---
> net/bluetooth/mgmt.c | 5 ++++-
> 5 files changed, 35 insertions(+), 6 deletions(-)
> ...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
>> This calls hci_update_background_scan() when there is any update on the
>> advertisement monitors. If there is at least one advertisement monitor,
>> the filtering policy of scan parameters should be 0x00. This also reports
>> device found mgmt events if there is at least one monitor.
>>
>> The following cases were tested with btmgmt advmon-* commands.
>> (1) add a ADV monitor and observe that the passive scanning is
>> triggered.
>> (2) remove the last ADV monitor and observe that the passive scanning is
>> terminated.
>> (3) with a LE peripheral paired, repeat (1) and observe the passive
>> scanning continues.
>> (4) with a LE peripheral paired, repeat (2) and observe the passive
>> scanning continues.
>> (5) with a ADV monitor, suspend/resume the host and observe the passive
>> scanning continues.
>>
>> Signed-off-by: Miao-chen Chou <[email protected]>
>> Signed-off-by: Marcel Holtmann <[email protected]>
>
> This patch landed recently in linux-next as commit 8208f5a9d435
> ("Bluetooth: Update background scan and report device based on
> advertisement monitors").
>
> It causes a regression, a kernel oops during system suspend/resume cycle
> on Samsung Exynos5250 based Snow Chromebook:
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = 86c149f5
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in: cmac cros_ec_sysfs cros_ec_lightbar cros_ec_debugfs
> cros_ec_chardev cros_ec_keyb cros_ec_dev snd_soc_hdmi_codec cros_ec_i2c
> cros_ec snd_soc_snow snd_soc_i2s snd_soc_idma snd_soc_s3c_dma exynosdrm
> analogix_dp exynos_gsc v4l2_mem2mem snd_soc_max98095 snd_soc_core
> ac97_bus snd_pcm_dmaengine snd_pcm snd_timer nxp_ptn3460 snd soundcore
> pwm_samsung spi_s3c64xx cyapatp crc_itu_t mwifiex_sdio mwifiex
> sha256_generic libsha256 sha256_arm btmrvl_sdio btmrvl cfg80211
> bluetooth s5p_mfc governor_simpleondemand videobuf2_dma_contig
> videobuf2_memops videobuf2_v4l2 ecdh_generic ecc videobuf2_common
> videodev phy_exynos_usb2 ohci_exynos panfrost gpu_sched mc s3c2410_wdt
> s5p_sss s5p_cec exynos_rng rtc_s3c i2c_arb_gpio_challenge
> CPU: 1 PID: 16 Comm: kworker/1:0 Not tainted
> 5.7.0-rc7-02995-g8208f5a9d435 #8564
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: events_freezable mmc_rescan
> PC is at __queue_work+0x6c/0x4e8
> LR is at __queue_work+0x68/0x4e8
> pc : [<c03619d8>] lr : [<c03619d4>] psr: 60000093
> ...
> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 6be2406a DAC: 00000051
> Process kworker/1:0 (pid: 16, stack limit = 0xf9898f75)
> Stack: (0xee117de0 to 0xee118000)
> ...
> [<c03619d8>] (__queue_work) from [<c0361e94>] (queue_work_on+0x40/0x4c)
> [<c0361e94>] (queue_work_on) from [<bf17dd0c>]
> (hci_adv_monitors_clear+0x74/0x90 [bluetooth])
> [<bf17dd0c>] (hci_adv_monitors_clear [bluetooth]) from [<bf17e138>]
> (hci_unregister_dev+0x158/0x224 [bluetooth])
> [<bf17e138>] (hci_unregister_dev [bluetooth]) from [<bf1d1638>]
> (btmrvl_remove_card+0x58/0x7c [btmrvl])
> [<bf1d1638>] (btmrvl_remove_card [btmrvl]) from [<c0d09698>]
> (sdio_bus_remove+0x30/0x11c)
> [<c0d09698>] (sdio_bus_remove) from [<c09d534c>]
> (device_release_driver_internal+0xe8/0x1ac)
> [<c09d534c>] (device_release_driver_internal) from [<c09d3e18>]
> (bus_remove_device+0xcc/0xf8)
> [<c09d3e18>] (bus_remove_device) from [<c09cfa3c>] (device_del+0x15c/0x384)
> [<c09cfa3c>] (device_del) from [<c0d098b8>] (sdio_remove_func+0x20/0x34)
> [<c0d098b8>] (sdio_remove_func) from [<c0d075bc>]
> (mmc_sdio_remove+0x38/0x64)
> [<c0d075bc>] (mmc_sdio_remove) from [<c0d08858>] (mmc_sdio_detect+0x6c/0xf8)
> [<c0d08858>] (mmc_sdio_detect) from [<c0cff6f0>] (mmc_rescan+0x1d0/0x42c)
> [<c0cff6f0>] (mmc_rescan) from [<c0362454>] (process_one_work+0x178/0x4ac)
> [<c0362454>] (process_one_work) from [<c0362b44>] (worker_thread+0x2c/0x530)
> [<c0362b44>] (worker_thread) from [<c0368610>] (kthread+0x12c/0x158)
> [<c0368610>] (kthread) from [<c03001a8>] (ret_from_fork+0x14/0x2c)
> Exception stack(0xee117fb0 to 0xee117ff8)
> ...
> ---[ end trace 0ec00d142e0a49cf ]---
>
> This board uses btmrvl_sdio bluetooth driver if that helps. Reverting
> this commit in linux-next 20200629 'fixes' the issue.
>
> I can do more tests if needed on this hardware, just let me know how can
> I help and what to do.
can you check latest bluetooth-next. I think that we applied a fix for it.
Regards
Marcel
Hi Marcel,
On 30.06.2020 08:49, Marcel Holtmann wrote:
>>> This calls hci_update_background_scan() when there is any update on the
>>> advertisement monitors. If there is at least one advertisement monitor,
>>> the filtering policy of scan parameters should be 0x00. This also reports
>>> device found mgmt events if there is at least one monitor.
>>>
>>> The following cases were tested with btmgmt advmon-* commands.
>>> (1) add a ADV monitor and observe that the passive scanning is
>>> triggered.
>>> (2) remove the last ADV monitor and observe that the passive scanning is
>>> terminated.
>>> (3) with a LE peripheral paired, repeat (1) and observe the passive
>>> scanning continues.
>>> (4) with a LE peripheral paired, repeat (2) and observe the passive
>>> scanning continues.
>>> (5) with a ADV monitor, suspend/resume the host and observe the passive
>>> scanning continues.
>>>
>>> Signed-off-by: Miao-chen Chou <[email protected]>
>>> Signed-off-by: Marcel Holtmann <[email protected]>
>> This patch landed recently in linux-next as commit 8208f5a9d435
>> ("Bluetooth: Update background scan and report device based on
>> advertisement monitors").
>>
>> It causes a regression, a kernel oops during system suspend/resume cycle
>> on Samsung Exynos5250 based Snow Chromebook:
>>
>> 8<--- cut here ---
>> Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> pgd = 86c149f5
>> [00000000] *pgd=00000000
>> Internal error: Oops: 5 [#1] SMP ARM
>> Modules linked in: cmac cros_ec_sysfs cros_ec_lightbar cros_ec_debugfs
>> cros_ec_chardev cros_ec_keyb cros_ec_dev snd_soc_hdmi_codec cros_ec_i2c
>> cros_ec snd_soc_snow snd_soc_i2s snd_soc_idma snd_soc_s3c_dma exynosdrm
>> analogix_dp exynos_gsc v4l2_mem2mem snd_soc_max98095 snd_soc_core
>> ac97_bus snd_pcm_dmaengine snd_pcm snd_timer nxp_ptn3460 snd soundcore
>> pwm_samsung spi_s3c64xx cyapatp crc_itu_t mwifiex_sdio mwifiex
>> sha256_generic libsha256 sha256_arm btmrvl_sdio btmrvl cfg80211
>> bluetooth s5p_mfc governor_simpleondemand videobuf2_dma_contig
>> videobuf2_memops videobuf2_v4l2 ecdh_generic ecc videobuf2_common
>> videodev phy_exynos_usb2 ohci_exynos panfrost gpu_sched mc s3c2410_wdt
>> s5p_sss s5p_cec exynos_rng rtc_s3c i2c_arb_gpio_challenge
>> CPU: 1 PID: 16 Comm: kworker/1:0 Not tainted
>> 5.7.0-rc7-02995-g8208f5a9d435 #8564
>> Hardware name: Samsung Exynos (Flattened Device Tree)
>> Workqueue: events_freezable mmc_rescan
>> PC is at __queue_work+0x6c/0x4e8
>> LR is at __queue_work+0x68/0x4e8
>> pc : [<c03619d8>] lr : [<c03619d4>] psr: 60000093
>> ...
>> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
>> Control: 10c5387d Table: 6be2406a DAC: 00000051
>> Process kworker/1:0 (pid: 16, stack limit = 0xf9898f75)
>> Stack: (0xee117de0 to 0xee118000)
>> ...
>> [<c03619d8>] (__queue_work) from [<c0361e94>] (queue_work_on+0x40/0x4c)
>> [<c0361e94>] (queue_work_on) from [<bf17dd0c>]
>> (hci_adv_monitors_clear+0x74/0x90 [bluetooth])
>> [<bf17dd0c>] (hci_adv_monitors_clear [bluetooth]) from [<bf17e138>]
>> (hci_unregister_dev+0x158/0x224 [bluetooth])
>> [<bf17e138>] (hci_unregister_dev [bluetooth]) from [<bf1d1638>]
>> (btmrvl_remove_card+0x58/0x7c [btmrvl])
>> [<bf1d1638>] (btmrvl_remove_card [btmrvl]) from [<c0d09698>]
>> (sdio_bus_remove+0x30/0x11c)
>> [<c0d09698>] (sdio_bus_remove) from [<c09d534c>]
>> (device_release_driver_internal+0xe8/0x1ac)
>> [<c09d534c>] (device_release_driver_internal) from [<c09d3e18>]
>> (bus_remove_device+0xcc/0xf8)
>> [<c09d3e18>] (bus_remove_device) from [<c09cfa3c>] (device_del+0x15c/0x384)
>> [<c09cfa3c>] (device_del) from [<c0d098b8>] (sdio_remove_func+0x20/0x34)
>> [<c0d098b8>] (sdio_remove_func) from [<c0d075bc>]
>> (mmc_sdio_remove+0x38/0x64)
>> [<c0d075bc>] (mmc_sdio_remove) from [<c0d08858>] (mmc_sdio_detect+0x6c/0xf8)
>> [<c0d08858>] (mmc_sdio_detect) from [<c0cff6f0>] (mmc_rescan+0x1d0/0x42c)
>> [<c0cff6f0>] (mmc_rescan) from [<c0362454>] (process_one_work+0x178/0x4ac)
>> [<c0362454>] (process_one_work) from [<c0362b44>] (worker_thread+0x2c/0x530)
>> [<c0362b44>] (worker_thread) from [<c0368610>] (kthread+0x12c/0x158)
>> [<c0368610>] (kthread) from [<c03001a8>] (ret_from_fork+0x14/0x2c)
>> Exception stack(0xee117fb0 to 0xee117ff8)
>> ...
>> ---[ end trace 0ec00d142e0a49cf ]---
>>
>> This board uses btmrvl_sdio bluetooth driver if that helps. Reverting
>> this commit in linux-next 20200629 'fixes' the issue.
>>
>> I can do more tests if needed on this hardware, just let me know how can
>> I help and what to do.
> can you check latest bluetooth-next. I think that we applied a fix for it.
Indeed. Commit f2790f025a0e ("Bluetooth: fix kernel null pointer
dereference error on suspend") fixed the issue.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland