michaelh/zephyr
1
0
Fork 0
Code Releases Activity

Bluetooth: host: Remove cancel sync from database hash commit

Browse source 
Fix deadlock when db_hash_commit has to wait for the delayed work to
finish. This creates a deadlock if the delayed work for database hash
calculation needs to store the hash since the settings API is locked
when calling the commit callback.
Remove call to k_work_cancel_delayable_sync from db_hash_commit in order
to avoid the deadlock. Instead move comparing of the stored hash to the
delayed work and reschedule the work with no wait.

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This commit is contained in:
Joakim Andersson 2021-05-18 16:46:26 +02:00 • committed by Anas Nashif
commit 7986cae05c
1 changed files with 47 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

82
subsys/bluetooth/host/gatt.c
View file

@ -229,6 +229,7 @@ enum {
#if defined(CONFIG_BT_GATT_CACHING)
DB_HASH_VALID, /* Database hash needs to be calculated */
DB_HASH_LOAD, /* Database hash loaded from settings. */
#endif
/* Total number of flags - must be at the end of the enum */
SC_NUM_FLAGS,
@ -248,6 +249,9 @@ static struct gatt_sc {
#if defined(CONFIG_BT_GATT_CACHING)
static struct db_hash {
uint8_t hash[16];
#if defined(CONFIG_BT_SETTINGS)
uint8_t stored_hash[16];
#endif
struct k_work_delayable work;
struct k_work_sync sync;
} db_hash;
@ -719,8 +723,44 @@ static void db_hash_gen(bool store)
atomic_set_bit(gatt_sc.flags, DB_HASH_VALID);
}
#if defined(CONFIG_BT_SETTINGS)
static void sc_indicate(uint16_t start, uint16_t end);
#endif
static void db_hash_process(struct k_work *work)
{
#if defined(CONFIG_BT_SETTINGS)
if (atomic_test_and_clear_bit(gatt_sc.flags, DB_HASH_LOAD)) {
if (!atomic_test_bit(gatt_sc.flags, DB_HASH_VALID)) {
db_hash_gen(false);
}
/* Check if hash matches then skip SC update */
if (!memcmp(db_hash.stored_hash, db_hash.hash,
sizeof(db_hash.stored_hash))) {
BT_DBG("Database Hash matches");
k_work_cancel_delayable(&gatt_sc.work);
atomic_clear_bit(gatt_sc.flags, SC_RANGE_CHANGED);
return;
}
BT_HEXDUMP_DBG(db_hash.hash, sizeof(db_hash.hash),
"New Hash: ");
/* GATT database has been modified since last boot, likely due
* to a firmware update or a dynamic service that was not
* re-registered on boot.
* Indicate Service Changed to all bonded devices for the full
* database range to invalidate client-side cache and force
* discovery on reconnect.
*/
sc_indicate(0x0001, 0xffff);
/* Hash did not match, overwrite with current hash */
db_hash_store();
return;
}
#endif /* defined(CONFIG_BT_SETTINGS) */
db_hash_gen(true);
}
@ -5147,58 +5187,30 @@ static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb,
SETTINGS_STATIC_HANDLER_DEFINE(bt_cf, "bt/cf", NULL, cf_set, NULL, NULL);
static uint8_t stored_hash[16];
static int db_hash_set(const char *name, size_t len_rd,
settings_read_cb read_cb, void *cb_arg)
{
ssize_t len;
len = read_cb(cb_arg, stored_hash, sizeof(stored_hash));
len = read_cb(cb_arg, db_hash.stored_hash, sizeof(db_hash.stored_hash));
if (len < 0) {
BT_ERR("Failed to decode value (err %zd)", len);
return len;
}
BT_HEXDUMP_DBG(stored_hash, sizeof(stored_hash), "Stored Hash: ");
BT_HEXDUMP_DBG(db_hash.stored_hash, sizeof(db_hash.stored_hash),
"Stored Hash: ");
return 0;
}
static int db_hash_commit(void)
{
k_sched_lock();
/* Stop work and generate the hash */
(void)k_work_cancel_delayable_sync(&db_hash.work, &db_hash.sync);
if (!atomic_test_bit(gatt_sc.flags, DB_HASH_VALID)) {
db_hash_gen(false);
}
k_sched_unlock();
/* Check if hash matches then skip SC update */
if (!memcmp(stored_hash, db_hash.hash, sizeof(stored_hash))) {
BT_DBG("Database Hash matches");
k_work_cancel_delayable(&gatt_sc.work);
atomic_clear_bit(gatt_sc.flags, SC_RANGE_CHANGED);
return 0;
}
BT_HEXDUMP_DBG(db_hash.hash, sizeof(db_hash.hash), "New Hash: ");
/**
* GATT database has been modified since last boot, likely due to
* a firmware update or a dynamic service that was not re-registered on
* boot. Indicate Service Changed to all bonded devices for the full
* database range to invalidate client-side cache and force discovery on
* reconnect.
atomic_set_bit(gatt_sc.flags, DB_HASH_LOAD);
/* Reschedule work to calculate and compare against the Hash value
* loaded from flash.
*/
sc_indicate(0x0001, 0xffff);
/* Hash did not match overwrite with current hash */
db_hash_store();
k_work_reschedule(&db_hash.work, K_NO_WAIT);
return 0;
}