linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue
@ 2024-04-24 14:07 Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe Vlad Pruteanu
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Vlad Pruteanu @ 2024-04-24 14:07 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: claudia.rosu, mihai-octavian.urzica, silviu.barbulescu,
	iulia.tanasescu, andrei.istodorescu, luiz.dentz, Vlad Pruteanu

This replaces the current implementation that uses a global
bcast_pa_requestsqueue with one that runs on a per adapter basis. In order
to implement this the btd_adapter field of bap_data has been replaced by a
new struct, bap_adapter, which includes btd_adapter, bcast_pa_req queue
and pa_timer_id.

Vlad Pruteanu (4):
  bap: Initialize bap_data for scanned device in bap_bcast_probe
  bap: Replace adapter in bap_data with bap_adapter
  bap: Improve handling of pa_req timer
  bap: Remove deleted devices from pa_req queue

 profiles/audio/bap.c | 196 +++++++++++++++++++++++++++++--------------
 1 file changed, 131 insertions(+), 65 deletions(-)

-- 
2.40.1


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

* [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe
  2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
@ 2024-04-24 14:07 ` Vlad Pruteanu
  2024-04-24 16:59   ` bap: Replace the global bcast_pa_requests with a per adapter queue bluez.test.bot
  2024-04-24 14:07 ` [PATCH BlueZ v2 2/4] bap: Replace adapter in bap_data with bap_adapter Vlad Pruteanu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Vlad Pruteanu @ 2024-04-24 14:07 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: claudia.rosu, mihai-octavian.urzica, silviu.barbulescu,
	iulia.tanasescu, andrei.istodorescu, luiz.dentz, Vlad Pruteanu

By moving the bap_data initialization for a scanned device to
bap_adapter_probe() the adapter field of bap_data will already be set when
short_lived_pa_sync is called. When adapter will be changed for
bap_adapter, this will help eliminate an additional queue search (in
short_lived_pa_sync).
---
 profiles/audio/bap.c | 62 ++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index ff6d6d881..9e93906ca 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2768,43 +2768,13 @@ static void bap_detached(struct bt_bap *bap, void *user_data)
 static int short_lived_pa_sync(struct bap_bcast_pa_req *req)
 {
 	struct btd_service *service = req->data.service;
-	struct btd_device *device = btd_service_get_device(service);
-	struct btd_adapter *adapter = device_get_adapter(device);
-	struct btd_gatt_database *database = btd_adapter_get_database(adapter);
 	struct bap_data *data = btd_service_get_user_data(service);
 	GError *err = NULL;
 
-	if (data) {
+	if (data->listen_io) {
 		DBG("Already probed");
 		return -1;
 	}
-	data = bap_data_new(device);
-	data->service = service;
-	data->adapter = adapter;
-	data->device = device;
-	data->bap = bt_bap_new(btd_gatt_database_get_db(database),
-			btd_gatt_database_get_db(database));
-	if (!data->bap) {
-		error("Unable to create BAP instance");
-		free(data);
-		return -EINVAL;
-	}
-
-	if (!bt_bap_attach(data->bap, NULL)) {
-		error("BAP unable to attach");
-		return -EINVAL;
-	}
-
-	bap_data_add(data);
-
-	data->ready_id = bt_bap_ready_register(data->bap, bap_ready, service,
-								NULL);
-	data->state_id = bt_bap_state_register(data->bap, bap_state_bcast,
-					bap_connecting_bcast, data, NULL);
-	data->pac_id = bt_bap_pac_register(data->bap, pac_added_broadcast,
-				pac_removed_broadcast, data, NULL);
-
-	bt_bap_set_user_data(data->bap, service);
 
 	DBG("Create PA sync with this source");
 	req->in_progress = TRUE;
@@ -2925,14 +2895,44 @@ static int bap_bcast_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
 	struct btd_adapter *adapter = device_get_adapter(device);
+	struct btd_gatt_database *database = btd_adapter_get_database(adapter);
 	struct bap_bcast_pa_req *pa_req =
 			new0(struct bap_bcast_pa_req, 1);
+	struct bap_data *data;
 
 	if (!btd_adapter_has_exp_feature(adapter, EXP_FEAT_ISO_SOCKET)) {
 		error("BAP requires ISO Socket which is not enabled");
 		return -ENOTSUP;
 	}
 
+	data = bap_data_new(device);
+	data->service = service;
+	data->adapter = adapter;
+	data->device = device;
+	data->bap = bt_bap_new(btd_gatt_database_get_db(database),
+			btd_gatt_database_get_db(database));
+	if (!data->bap) {
+		error("Unable to create BAP instance");
+		free(data);
+		return -EINVAL;
+	}
+
+	if (!bt_bap_attach(data->bap, NULL)) {
+		error("BAP unable to attach");
+		return -EINVAL;
+	}
+
+	bap_data_add(data);
+
+	data->ready_id = bt_bap_ready_register(data->bap, bap_ready, service,
+								NULL);
+	data->state_id = bt_bap_state_register(data->bap, bap_state_bcast,
+					bap_connecting_bcast, data, NULL);
+	data->pac_id = bt_bap_pac_register(data->bap, pac_added_broadcast,
+				pac_removed_broadcast, data, NULL);
+
+	bt_bap_set_user_data(data->bap, service);
+
 	/* First time initialize the queue and start the idle timer */
 	if (bcast_pa_requests == NULL) {
 		bcast_pa_requests = queue_new();
-- 
2.40.1


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

* [PATCH BlueZ v2 2/4] bap: Replace adapter in bap_data with bap_adapter
  2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe Vlad Pruteanu
@ 2024-04-24 14:07 ` Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 3/4] bap: Improve handling of pa_req timer Vlad Pruteanu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Vlad Pruteanu @ 2024-04-24 14:07 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: claudia.rosu, mihai-octavian.urzica, silviu.barbulescu,
	iulia.tanasescu, andrei.istodorescu, luiz.dentz, Vlad Pruteanu

This patch introduces the bap_adapter structure. In addition to btd_adapter
it also holds the pa_timer_id and the bcast_pa_requests queue associated
with that adapter. This enables convenient access to these variables since
the functions that need them already utilize bap_data.

For each adapter a new instance of bap_adapter is created and inserted into
the global queue, bap_adapters.

For each scanned source bap_bcast_probe searches the bap_adapters queue
based on the adapter and stores the result in the bap_data associated
with the source. Operations made on the old global queue are now made
on bap_data->bap_adapter->bcast_pa_requests queue.

While this commit sought to utilize the already existing bap_data in order
to avoid searching in queues, a lookup was still necessary in
bap_bcast_probe. Here, the bap_data for the scanned devices is created and
the bap_adapter field must be set to the appropriate value. There is no way
of getting the correct bap_adapter refference without searching the
bap_adapters queue.
---
 profiles/audio/bap.c | 100 ++++++++++++++++++++++++++++---------------
 1 file changed, 66 insertions(+), 34 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 9e93906ca..d8cd05f26 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -98,9 +98,15 @@ struct bap_ep {
 	struct queue *setups;
 };
 
+struct bap_adapter {
+	struct btd_adapter *adapter;
+	unsigned int pa_timer_id;
+	struct queue *bcast_pa_requests;
+};
+
 struct bap_data {
 	struct btd_device *device;
-	struct btd_adapter *adapter;
+	struct bap_adapter *bap_adapter;
 	struct btd_service *service;
 	struct bt_bap *bap;
 	unsigned int ready_id;
@@ -130,8 +136,7 @@ struct bap_bcast_pa_req {
 };
 
 static struct queue *sessions;
-static struct queue *bcast_pa_requests;
-static unsigned int pa_timer_id;
+static struct queue *bap_adapters;
 
 /* Structure holding the parameters for Periodic Advertisement create sync.
  * The full QOS is populated at the time the user selects and endpoint and
@@ -366,7 +371,7 @@ static gboolean get_device(const GDBusPropertyTable *property,
 	const char *path;
 
 	if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE)
-		path = adapter_get_path(ep->data->adapter);
+		path = adapter_get_path(ep->data->bap_adapter->adapter);
 	else
 		path = device_get_path(ep->data->device);
 
@@ -993,7 +998,7 @@ static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data)
 
 	DBG("BIG Sync completed");
 
-	queue_remove(bcast_pa_requests, req);
+	queue_remove(setup->ep->data->bap_adapter->bcast_pa_requests, req);
 
 	/* This device is no longer needed */
 	btd_service_connecting_complete(setup->ep->data->service, 0);
@@ -1150,8 +1155,8 @@ static void iso_pa_sync_confirm_cb(GIOChannel *io, void *user_data)
 	g_io_channel_unref(data->listen_io);
 	g_io_channel_shutdown(io, TRUE, NULL);
 	data->listen_io = NULL;
-	queue_remove(bcast_pa_requests, pa_req);
-
+	queue_remove(data->bap_adapter->bcast_pa_requests, pa_req);
+	free(pa_req);
 	/* Analyze received BASE data and create remote media endpoints for each
 	 * BIS matching our capabilities
 	 */
@@ -1209,7 +1214,7 @@ static struct bap_ep *ep_register_bcast(struct bap_data *data,
 					struct bt_bap_pac *lpac,
 					struct bt_bap_pac *rpac)
 {
-	struct btd_adapter *adapter = data->adapter;
+	struct btd_adapter *adapter = data->bap_adapter->adapter;
 	struct btd_device *device = data->device;
 	struct bap_ep *ep;
 	struct queue *queue;
@@ -2062,14 +2067,15 @@ static void pa_and_big_sync(struct bap_bcast_pa_req *req);
 
 static gboolean pa_idle_timer(gpointer user_data)
 {
-	struct bap_bcast_pa_req *req = user_data;
+	struct bap_adapter *bap_adapter = user_data;
+	struct bap_bcast_pa_req *req;
 	bool in_progress = FALSE;
 
 	/* Handle timer if no request is in progress */
-	queue_foreach(bcast_pa_requests, check_pa_req_in_progress,
+	queue_foreach(bap_adapter->bcast_pa_requests, check_pa_req_in_progress,
 			&in_progress);
 	if (in_progress == FALSE) {
-		req = queue_peek_head(bcast_pa_requests);
+		req = queue_peek_head(bap_adapter->bcast_pa_requests);
 		if (req != NULL)
 			switch (req->type) {
 			case BAP_PA_SHORT_REQ:
@@ -2090,6 +2096,7 @@ static void setup_accept_io_broadcast(struct bap_data *data,
 					struct bap_setup *setup)
 {
 	struct bap_bcast_pa_req *pa_req = new0(struct bap_bcast_pa_req, 1);
+	struct bap_adapter *bap_adapter = data->bap_adapter;
 
 	/* Add this request to the PA queue.
 	 * We don't need to check the queue here and the timer, as we cannot
@@ -2098,7 +2105,7 @@ static void setup_accept_io_broadcast(struct bap_data *data,
 	pa_req->type = BAP_PA_BIG_SYNC_REQ;
 	pa_req->in_progress = FALSE;
 	pa_req->data.setup = setup;
-	queue_push_tail(bcast_pa_requests, pa_req);
+	queue_push_tail(bap_adapter->bcast_pa_requests, pa_req);
 }
 
 static void setup_create_ucast_io(struct bap_data *data,
@@ -2779,18 +2786,18 @@ static int short_lived_pa_sync(struct bap_bcast_pa_req *req)
 	DBG("Create PA sync with this source");
 	req->in_progress = TRUE;
 	data->listen_io = bt_io_listen(NULL, iso_pa_sync_confirm_cb, req,
-			NULL, &err,
-			BT_IO_OPT_SOURCE_BDADDR,
-			btd_adapter_get_address(data->adapter),
-			BT_IO_OPT_SOURCE_TYPE,
-			btd_adapter_get_address_type(data->adapter),
-			BT_IO_OPT_DEST_BDADDR,
-			device_get_address(data->device),
-			BT_IO_OPT_DEST_TYPE,
-			btd_device_get_bdaddr_type(data->device),
-			BT_IO_OPT_MODE, BT_IO_MODE_ISO,
-			BT_IO_OPT_QOS, &bap_sink_pa_qos,
-			BT_IO_OPT_INVALID);
+		NULL, &err,
+		BT_IO_OPT_SOURCE_BDADDR,
+		btd_adapter_get_address(data->bap_adapter->adapter),
+		BT_IO_OPT_SOURCE_TYPE,
+		btd_adapter_get_address_type(data->bap_adapter->adapter),
+		BT_IO_OPT_DEST_BDADDR,
+		device_get_address(data->device),
+		BT_IO_OPT_DEST_TYPE,
+		btd_device_get_bdaddr_type(data->device),
+		BT_IO_OPT_MODE, BT_IO_MODE_ISO,
+		BT_IO_OPT_QOS, &bap_sink_pa_qos,
+		BT_IO_OPT_INVALID);
 	if (!data->listen_io) {
 		error("%s", err->message);
 		g_error_free(err);
@@ -2877,7 +2884,7 @@ static void pa_and_big_sync(struct bap_bcast_pa_req *req)
 	setup->io = bt_io_listen(NULL, iso_do_big_sync, req,
 			NULL, &err,
 			BT_IO_OPT_SOURCE_BDADDR,
-			btd_adapter_get_address(data->adapter),
+			btd_adapter_get_address(data->bap_adapter->adapter),
 			BT_IO_OPT_DEST_BDADDR,
 			device_get_address(data->device),
 			BT_IO_OPT_DEST_TYPE,
@@ -2891,11 +2898,20 @@ static void pa_and_big_sync(struct bap_bcast_pa_req *req)
 	}
 }
 
+static bool match_bap_adapter(const void *data, const void *match_data)
+{
+	struct bap_adapter *bap_adapter = (struct bap_adapter *)data;
+
+	return bap_adapter->adapter == match_data;
+}
+
 static int bap_bcast_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
 	struct btd_adapter *adapter = device_get_adapter(device);
 	struct btd_gatt_database *database = btd_adapter_get_database(adapter);
+	struct bap_adapter *bap_adapter = queue_find(bap_adapters,
+						match_bap_adapter, adapter);
 	struct bap_bcast_pa_req *pa_req =
 			new0(struct bap_bcast_pa_req, 1);
 	struct bap_data *data;
@@ -2907,7 +2923,7 @@ static int bap_bcast_probe(struct btd_service *service)
 
 	data = bap_data_new(device);
 	data->service = service;
-	data->adapter = adapter;
+	data->bap_adapter = bap_adapter;
 	data->device = device;
 	data->bap = bt_bap_new(btd_gatt_database_get_db(database),
 			btd_gatt_database_get_db(database));
@@ -2933,12 +2949,10 @@ static int bap_bcast_probe(struct btd_service *service)
 
 	bt_bap_set_user_data(data->bap, service);
 
-	/* First time initialize the queue and start the idle timer */
-	if (bcast_pa_requests == NULL) {
-		bcast_pa_requests = queue_new();
-		pa_timer_id = g_timeout_add_seconds(PA_IDLE_TIMEOUT,
-					pa_idle_timer, NULL);
-	}
+	/* Start the PA timer if it hasn't been started yet */
+	if (bap_adapter->pa_timer_id == 0)
+		bap_adapter->pa_timer_id = g_timeout_add_seconds(
+		PA_IDLE_TIMEOUT, pa_idle_timer, bap_adapter);
 
 	/* Enqueue this device advertisement so that we can do short-lived
 	 */
@@ -2946,7 +2960,7 @@ static int bap_bcast_probe(struct btd_service *service)
 	pa_req->type = BAP_PA_SHORT_REQ;
 	pa_req->in_progress = FALSE;
 	pa_req->data.service = service;
-	queue_push_tail(bcast_pa_requests, pa_req);
+	queue_push_tail(bap_adapter->bcast_pa_requests, pa_req);
 
 	return 0;
 }
@@ -3067,6 +3081,7 @@ static int bap_adapter_probe(struct btd_profile *p,
 {
 	struct btd_gatt_database *database = btd_adapter_get_database(adapter);
 	struct bap_data *data;
+	struct bap_adapter *bap_adapter;
 	char addr[18];
 
 	ba2str(btd_adapter_get_address(adapter), addr);
@@ -3078,7 +3093,6 @@ static int bap_adapter_probe(struct btd_profile *p,
 	}
 
 	data = bap_data_new(NULL);
-	data->adapter = adapter;
 
 	data->bap = bt_bap_new(btd_gatt_database_get_db(database),
 					btd_gatt_database_get_db(database));
@@ -3102,6 +3116,15 @@ static int bap_adapter_probe(struct btd_profile *p,
 
 	bt_bap_set_user_data(data->bap, adapter);
 	bap_data_set_user_data(data, adapter);
+
+	bap_adapter = new0(struct bap_adapter, 1);
+	bap_adapter->adapter = adapter;
+	data->bap_adapter = bap_adapter;
+
+	if (bap_adapters == NULL)
+		bap_adapters = queue_new();
+	bap_adapter->bcast_pa_requests = queue_new();
+	queue_push_tail(bap_adapters, bap_adapter);
 	return 0;
 }
 
@@ -3115,6 +3138,15 @@ static void bap_adapter_remove(struct btd_profile *p,
 	ba2str(btd_adapter_get_address(adapter), addr);
 	DBG("%s", addr);
 
+	queue_destroy(data->bap_adapter->bcast_pa_requests, free);
+	queue_remove(bap_adapters, data->bap_adapter);
+	free(data->bap_adapter);
+
+	if (queue_isempty(bap_adapters)) {
+		queue_destroy(bap_adapters, NULL);
+		bap_adapters = NULL;
+	}
+
 	if (!data) {
 		error("BAP service not handled by profile");
 		return;
-- 
2.40.1


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

* [PATCH BlueZ v2 3/4] bap: Improve handling of pa_req timer
  2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 2/4] bap: Replace adapter in bap_data with bap_adapter Vlad Pruteanu
@ 2024-04-24 14:07 ` Vlad Pruteanu
  2024-04-24 14:07 ` [PATCH BlueZ v2 4/4] bap: Remove deleted devices from pa_req queue Vlad Pruteanu
  2024-04-24 19:10 ` [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: Vlad Pruteanu @ 2024-04-24 14:07 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: claudia.rosu, mihai-octavian.urzica, silviu.barbulescu,
	iulia.tanasescu, andrei.istodorescu, luiz.dentz, Vlad Pruteanu

The patch handles timers on a per adapter basis.

The timer is now also started on setup_accept_io_broadcast, so
BAP_PA_BIG_SYNC_REQ can be treated if the timer is stopped in the meantime.
The timer is stopped if the bap_adapter's pa_req queue is empty.
A pa_timer_id equal to 0 means that the timer is stopped.
---
 profiles/audio/bap.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index d8cd05f26..ab0b96222 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2087,6 +2087,14 @@ static gboolean pa_idle_timer(gpointer user_data)
 				pa_and_big_sync(req);
 				break;
 			}
+		else {
+			/* pa_req queue is empty, stop the timer by returning
+			 * FALSE and set the pa_timer_id to 0. This will later
+			 * be used to check if the timer is active.
+			 */
+			bap_adapter->pa_timer_id = 0;
+			return FALSE;
+		}
 	}
 
 	return TRUE;
@@ -2098,9 +2106,17 @@ static void setup_accept_io_broadcast(struct bap_data *data,
 	struct bap_bcast_pa_req *pa_req = new0(struct bap_bcast_pa_req, 1);
 	struct bap_adapter *bap_adapter = data->bap_adapter;
 
+	/* Timer could be stopped if all the short lived requests were treated.
+	 * Check the state of the timer and turn it on so that this requests
+	 * can also be treated.
+	 */
+	if (bap_adapter->pa_timer_id == 0)
+		bap_adapter->pa_timer_id = g_timeout_add_seconds(
+		PA_IDLE_TIMEOUT, pa_idle_timer, bap_adapter);
+
 	/* Add this request to the PA queue.
-	 * We don't need to check the queue here and the timer, as we cannot
-	 * have BAP_PA_BIG_SYNC_REQ before a short PA (BAP_PA_SHORT_REQ)
+	 * We don't need to check the queue here, as we cannot have
+	 * BAP_PA_BIG_SYNC_REQ before a short PA (BAP_PA_SHORT_REQ)
 	 */
 	pa_req->type = BAP_PA_BIG_SYNC_REQ;
 	pa_req->in_progress = FALSE;
-- 
2.40.1


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

* [PATCH BlueZ v2 4/4] bap: Remove deleted devices from pa_req queue
  2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
                   ` (2 preceding siblings ...)
  2024-04-24 14:07 ` [PATCH BlueZ v2 3/4] bap: Improve handling of pa_req timer Vlad Pruteanu
@ 2024-04-24 14:07 ` Vlad Pruteanu
  2024-04-24 19:10 ` [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: Vlad Pruteanu @ 2024-04-24 14:07 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: claudia.rosu, mihai-octavian.urzica, silviu.barbulescu,
	iulia.tanasescu, andrei.istodorescu, luiz.dentz, Vlad Pruteanu

The bap_bcast_remove function has been updated to remove from the
pa_req queue entries of devices that were freed. pa_req that are already
in progress are treated by the bap_data_free function.

The lookup in bap_bcast_remove was necessary. The entry corresponding to
the calling service must be removed from the pa_req queue. There is no
other way to get a refference to this entry other than to search in the
queue.

This patch fixes a crash that occurs when a device is freed before the
pa_idle_timer handles it's entry in the pa_req queue. The following log
was obtained while running an Unicast setup:

==105052==ERROR: AddressSanitizer: heap-use-after-free on address
0x60400001c418 at pc 0x55775caf1846 bp 0x7ffc83d9fb90 sp 0x7ffc83d9fb80
READ of size 8 at 0x60400001c418 thread T0
0 0x55775caf1845 in btd_service_get_device src/service.c:325
1 0x55775ca03da2 in short_lived_pa_sync profiles/audio/bap.c:2693
2 0x55775ca03da2 in pa_idle_timer profiles/audio/bap.c:1996
---
 profiles/audio/bap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index ab0b96222..2b1ed501b 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2981,10 +2981,20 @@ static int bap_bcast_probe(struct btd_service *service)
 	return 0;
 }
 
+static bool match_service(const void *data, const void *match_data)
+{
+	struct bap_bcast_pa_req *pa_req = (struct bap_bcast_pa_req *)data;
+
+	if (pa_req->data.service == match_data)
+		return true;
+	return false;
+}
+
 static void bap_bcast_remove(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
 	struct bap_data *data;
+	struct bap_bcast_pa_req *pa_req;
 	char addr[18];
 
 	ba2str(device_get_address(device), addr);
@@ -2995,6 +3005,14 @@ static void bap_bcast_remove(struct btd_service *service)
 		error("BAP service not handled by profile");
 		return;
 	}
+	/* Remove the corresponding entry from the pa_req queue. Any pa_req that
+	 * are in progress will be stopped by bap_data_remove which calls
+	 * bap_data_free.
+	 */
+	pa_req = queue_remove_if(data->bap_adapter->bcast_pa_requests,
+						match_service, service);
+	if (pa_req)
+		free(pa_req);
 
 	bap_data_remove(data);
 }
-- 
2.40.1


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

* RE: bap: Replace the global bcast_pa_requests with a per adapter queue
  2024-04-24 14:07 ` [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe Vlad Pruteanu
@ 2024-04-24 16:59   ` bluez.test.bot
  0 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2024-04-24 16:59 UTC (permalink / raw)
  To: linux-bluetooth, vlad.pruteanu

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=847482

---Test result---

Test Summary:
CheckPatch                    PASS      5.65 seconds
GitLint                       PASS      1.27 seconds
BuildEll                      PASS      24.27 seconds
BluezMake                     PASS      1678.41 seconds
MakeCheck                     PASS      13.02 seconds
MakeDistcheck                 PASS      176.27 seconds
CheckValgrind                 PASS      244.95 seconds
CheckSmatch                   PASS      348.74 seconds
bluezmakeextell               PASS      119.73 seconds
IncrementalBuild              PASS      6256.02 seconds
ScanBuild                     PASS      985.83 seconds



---
Regards,
Linux Bluetooth


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

* Re: [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue
  2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
                   ` (3 preceding siblings ...)
  2024-04-24 14:07 ` [PATCH BlueZ v2 4/4] bap: Remove deleted devices from pa_req queue Vlad Pruteanu
@ 2024-04-24 19:10 ` patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2024-04-24 19:10 UTC (permalink / raw)
  To: Vlad Pruteanu
  Cc: linux-bluetooth, claudia.rosu, mihai-octavian.urzica,
	silviu.barbulescu, iulia.tanasescu, andrei.istodorescu,
	luiz.dentz

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 24 Apr 2024 17:07:37 +0300 you wrote:
> This replaces the current implementation that uses a global
> bcast_pa_requestsqueue with one that runs on a per adapter basis. In order
> to implement this the btd_adapter field of bap_data has been replaced by a
> new struct, bap_adapter, which includes btd_adapter, bcast_pa_req queue
> and pa_timer_id.
> 
> Vlad Pruteanu (4):
>   bap: Initialize bap_data for scanned device in bap_bcast_probe
>   bap: Replace adapter in bap_data with bap_adapter
>   bap: Improve handling of pa_req timer
>   bap: Remove deleted devices from pa_req queue
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=638774f603c2
  - [BlueZ,v2,2/4] bap: Replace adapter in bap_data with bap_adapter
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=aa7f3574f275
  - [BlueZ,v2,3/4] bap: Improve handling of pa_req timer
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=48a69222581c
  - [BlueZ,v2,4/4] bap: Remove deleted devices from pa_req queue
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c7071911d57a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2024-04-24 19:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 14:07 [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue Vlad Pruteanu
2024-04-24 14:07 ` [PATCH BlueZ v2 1/4] bap: Initialize bap_data for scanned device in bap_bcast_probe Vlad Pruteanu
2024-04-24 16:59   ` bap: Replace the global bcast_pa_requests with a per adapter queue bluez.test.bot
2024-04-24 14:07 ` [PATCH BlueZ v2 2/4] bap: Replace adapter in bap_data with bap_adapter Vlad Pruteanu
2024-04-24 14:07 ` [PATCH BlueZ v2 3/4] bap: Improve handling of pa_req timer Vlad Pruteanu
2024-04-24 14:07 ` [PATCH BlueZ v2 4/4] bap: Remove deleted devices from pa_req queue Vlad Pruteanu
2024-04-24 19:10 ` [PATCH BlueZ v2 0/4] bap: Replace the global bcast_pa_requests with a per adapter queue patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).