Re: [PATCH v2 3/5] Bluetooth: advmon offload MSFT remove monitor

2020-12-15 Thread Archie Pusaka
Hi Dan,

On Wed, 16 Dec 2020 at 03:06, Dan Carpenter  wrote:
>
> Hi Archie,
>
> url:
> https://github.com/0day-ci/linux/commits/Archie-Pusaka/MSFT-offloading-support-for-advertisement-monitor/20201215-163858
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git 
> master
> config: i386-randconfig-m021-20201215 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> Reported-by: Dan Carpenter 
>
> smatch warnings:
> net/bluetooth/msft.h:45 msft_remove_monitor() warn: signedness bug returning 
> '(-95)'
>
> vim +45 net/bluetooth/msft.h
>
> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  41  static inline bool 
> msft_remove_monitor(struct hci_dev *hdev,
>  
> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  42 
>  struct adv_monitor *monitor,
> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  43 
>  u16 handle)
> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  44  {
> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15 @45   return -EOPNOTSUPP;
> ^^^
> change this to return false?
>
Thanks. Will fix.

> d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  46  }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Regards,
Archie


Re: [PATCH v2 3/5] Bluetooth: advmon offload MSFT remove monitor

2020-12-15 Thread Dan Carpenter
Hi Archie,

url:
https://github.com/0day-ci/linux/commits/Archie-Pusaka/MSFT-offloading-support-for-advertisement-monitor/20201215-163858
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git 
master
config: i386-randconfig-m021-20201215 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 
Reported-by: Dan Carpenter 

smatch warnings:
net/bluetooth/msft.h:45 msft_remove_monitor() warn: signedness bug returning 
'(-95)'

vim +45 net/bluetooth/msft.h

d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  41  static inline bool 
msft_remove_monitor(struct hci_dev *hdev,
 
d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  42  
struct adv_monitor *monitor,
d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  43  
u16 handle)
d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  44  {
d8ca4cbc63c57c8 Archie Pusaka  2020-12-15 @45   return -EOPNOTSUPP;
^^^
change this to return false?

d8ca4cbc63c57c8 Archie Pusaka  2020-12-15  46  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


[PATCH v2 3/5] Bluetooth: advmon offload MSFT remove monitor

2020-12-15 Thread Archie Pusaka
From: Archie Pusaka 

Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.

Signed-off-by: Archie Pusaka 
Reviewed-by: Miao-chen Chou 
Reviewed-by: Yun-Hao Chung 

---

(no changes since v1)

 include/net/bluetooth/hci_core.h |   8 +-
 net/bluetooth/hci_core.c | 119 +++--
 net/bluetooth/mgmt.c | 110 +-
 net/bluetooth/msft.c | 127 ++-
 net/bluetooth/msft.h |   9 +++
 5 files changed, 323 insertions(+), 50 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8 
instance);
 void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
 
 void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
 int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
 bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
 bool hci_is_adv_monitoring(struct hci_dev *hdev);
 int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
 
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct 
hci_dev *hdev,
u8 instance);
 void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
  u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
 int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
 int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
 
 u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
  u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fa13e35f775d..782b05ca0d0c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
 
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
-   hci_free_adv_monitor(monitor);
+   hci_free_adv_monitor(hdev, monitor);
 
idr_destroy(>adv_monitors_idr);
 }
 
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
 {
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
 
-   list_for_each_entry_safe(pattern, tmp, >patterns, list)
+   list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+   list_del(>list);
kfree(pattern);
+   }
+
+   if (monitor->handle)
+   idr_remove(>adv_monitors_idr, monitor->handle);
+
+   if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+   hdev->adv_monitors_cnt--;
+   mgmt_adv_monitor_removed(hdev, monitor->handle);
+   }
 
kfree(monitor);
 }
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev 
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
 }
 
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+   return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
 /* Assigns handle to a monitor, and if offloading is supported and power is on,
  * also attempts to forward the request to the controller.
  * Returns true if request is forwarded (result is pending), false otherwise.
@@ -3121,39 +3139,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct 
adv_monitor *monitor,
return (*err == 0);
 }
 
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+static bool hci_remove_adv_monitor(struct hci_dev *hdev,
+  struct adv_monitor