diff --git a/nimble/host/src/ble_gattc.c b/nimble/host/src/ble_gattc.c index b34b0dcfd..4f85ccc1a 100644 --- a/nimble/host/src/ble_gattc.c +++ b/nimble/host/src/ble_gattc.c @@ -112,6 +112,14 @@ /** Procedure stalled due to resource exhaustion. */ #define BLE_GATTC_PROC_F_STALLED 0x01 +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) +/** Procedure is still referenced by its public API caller. */ +#define BLE_GATTC_PROC_F_API_LOCKED 0x02 + +/** Procedure free was deferred until its public API caller returns. */ +#define BLE_GATTC_PROC_F_FREE_PENDING 0x04 +#endif + /** Represents an in-progress GATT procedure. */ struct ble_gattc_proc { STAILQ_ENTRY(ble_gattc_proc) next; @@ -246,10 +254,6 @@ struct ble_gattc_proc { STAILQ_HEAD(ble_gattc_proc_list, ble_gattc_proc); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) -static struct ble_gattc_proc_list temp_proc_list; -#endif - /** * Error functions - these handle an incoming ATT error response and apply it * to the appropriate active GATT procedure. @@ -874,6 +878,16 @@ ble_gattc_proc_free(struct ble_gattc_proc *proc) int i; if (proc != NULL) { +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + ble_hs_lock_nested(); + if (proc->flags & BLE_GATTC_PROC_F_API_LOCKED) { + proc->flags |= BLE_GATTC_PROC_F_FREE_PENDING; + ble_hs_unlock_nested(); + return; + } + ble_hs_unlock_nested(); +#endif + ble_gattc_dbg_assert_proc_not_inserted(proc); switch (proc->op) { @@ -940,15 +954,6 @@ ble_gattc_proc_insert(struct ble_gattc_proc *proc, bool insert_head) return; } } -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - /* Remove from temp_proc_list only if it is actually present. */ - STAILQ_FOREACH(cur, &temp_proc_list, next) { - if (cur == proc) { - STAILQ_REMOVE(&temp_proc_list, proc, ble_gattc_proc, next); - break; - } - } -#endif } if (insert_head) { @@ -960,33 +965,31 @@ ble_gattc_proc_insert(struct ble_gattc_proc *proc, bool insert_head) } #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) -static bool -ble_gattc_proc_remove_preempted(struct ble_gattc_proc *proc) +static int +ble_gattc_proc_remove_locked(struct ble_gattc_proc *proc) { struct ble_gattc_proc *cur; - bool removed = false; + struct ble_gattc_proc *prev; - ble_hs_lock(); - STAILQ_FOREACH(cur, &temp_proc_list, next) { + BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task()); + + prev = NULL; + cur = STAILQ_FIRST(&ble_gattc_procs); + while (cur != NULL) { if (cur == proc) { - STAILQ_REMOVE(&temp_proc_list, proc, ble_gattc_proc, next); - removed = true; - break; - } - } - - if (!removed) { - STAILQ_FOREACH(cur, &ble_gattc_procs, next) { - if (cur == proc) { - STAILQ_REMOVE(&ble_gattc_procs, proc, ble_gattc_proc, next); - removed = true; - break; + if (prev == NULL) { + STAILQ_REMOVE_HEAD(&ble_gattc_procs, next); + } else { + STAILQ_REMOVE_AFTER(&ble_gattc_procs, prev, next); } + return 1; } - } - ble_hs_unlock(); - return removed; + prev = cur; + cur = STAILQ_NEXT(cur, next); + } + + return 0; } #endif @@ -997,6 +1000,48 @@ ble_gattc_proc_set_exp_timer(struct ble_gattc_proc *proc) ble_npl_time_ms_to_ticks32(BLE_GATTC_UNRESPONSIVE_TIMEOUT_MS); } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) +static void +ble_gattc_proc_preempt_insert(struct ble_gattc_proc *proc) +{ + if (!(proc->flags & BLE_GATTC_PROC_F_STALLED)) { + ble_gattc_proc_set_exp_timer(proc); + } + + ble_hs_lock(); + proc->flags |= BLE_GATTC_PROC_F_API_LOCKED; + STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next); + ble_hs_unlock(); + + ble_hs_timer_resched(); +} + +static void +ble_gattc_proc_preempt_complete(struct ble_gattc_proc *proc, int status) +{ + int free_proc; + + free_proc = 0; + + ble_hs_lock(); + if (status != 0) { + free_proc = ble_gattc_proc_remove_locked(proc); + } + + if (proc->flags & BLE_GATTC_PROC_F_FREE_PENDING) { + proc->flags &= ~BLE_GATTC_PROC_F_FREE_PENDING; + free_proc = 1; + } + + proc->flags &= ~BLE_GATTC_PROC_F_API_LOCKED; + ble_hs_unlock(); + + if (free_proc) { + ble_gattc_proc_free(proc); + } +} +#endif + #if MYNEWT_VAL(BLE_GATTC) static void ble_gattc_proc_set_resume_timer(struct ble_gattc_proc *proc) @@ -1244,13 +1289,11 @@ ble_gattc_extract(ble_gattc_match_fn *cb, void *arg, int max_procs, struct ble_gattc_proc *proc; struct ble_gattc_proc *prev; struct ble_gattc_proc *next; -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - struct ble_gattc_proc *cur; - uint8_t flag = 0; -#endif int num_extracted; - /* Only the parent task is allowed to remove entries from the list. */ + /* Parent task performs normal extraction. The preemption-protect TX + * failure path may also remove an active proc while holding ble_hs_lock. + */ BLE_HS_DBG_ASSERT(ble_hs_is_parent_task()); STAILQ_INIT(dst_list); @@ -1260,27 +1303,6 @@ ble_gattc_extract(ble_gattc_match_fn *cb, void *arg, int max_procs, prev = NULL; -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - proc = STAILQ_FIRST(&temp_proc_list); - while (proc != NULL) { - next = STAILQ_NEXT(proc, next); - STAILQ_FOREACH(cur, &ble_gattc_procs, next) { - if (proc == cur) { - flag = 1; - break; - } - } - if (!flag) { - /* Detected a preemption case */ - STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next); - } - flag = 0; - proc = next; - } - /* Clear the temp proc list */ - STAILQ_INIT(&temp_proc_list); -#endif - proc = STAILQ_FIRST(&ble_gattc_procs); while (proc != NULL) { next = STAILQ_NEXT(proc, next); @@ -1918,7 +1940,9 @@ ble_gattc_disc_all_svcs_tx(struct ble_gattc_proc *proc) ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_PRIMARY_SERVICE); int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif rc = ble_att_clt_tx_read_group_type(proc->conn_handle, proc->cid, proc->disc_all_svcs.prev_handle + 1, @@ -2082,9 +2106,7 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb, proc->disc_all_svcs.cb_arg = cb_arg; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - ble_hs_unlock(); + ble_gattc_proc_preempt_insert(proc); #endif ble_gattc_log_proc_init("discover all services\n"); @@ -2096,15 +2118,15 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb, done: if (rc != 0) { STATS_INC(ble_gattc_stats, disc_all_svcs_fail); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (proc != NULL) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -2397,7 +2419,9 @@ ble_gattc_find_inc_svcs_tx(struct ble_gattc_proc *proc) ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_INCLUDE); int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif if (proc->find_inc_svcs.cur_start == 0) { if (proc->find_inc_svcs.prev_handle >= proc->find_inc_svcs.end_handle || @@ -2684,9 +2708,7 @@ ble_gattc_find_inc_svcs(uint16_t conn_handle, uint16_t start_handle, proc->find_inc_svcs.cb_arg = cb_arg; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - ble_hs_unlock(); + ble_gattc_proc_preempt_insert(proc); #endif ble_gattc_log_find_inc_svcs(proc); @@ -2698,15 +2720,15 @@ ble_gattc_find_inc_svcs(uint16_t conn_handle, uint16_t start_handle, done: if (rc != 0) { STATS_INC(ble_gattc_stats, find_inc_svcs_fail); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (proc != NULL) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -2766,7 +2788,9 @@ ble_gattc_disc_all_chrs_tx(struct ble_gattc_proc *proc) ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_CHARACTERISTIC); int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif rc = ble_att_clt_tx_read_type(proc->conn_handle, proc->cid, proc->disc_all_chrs.prev_handle + 1, @@ -2943,12 +2967,10 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle, proc->disc_all_chrs.cb = cb; proc->disc_all_chrs.cb_arg = cb_arg; -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - ble_hs_unlock(); -#endif ble_gattc_log_disc_all_chrs(proc); +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + ble_gattc_proc_preempt_insert(proc); +#endif rc = ble_gattc_disc_all_chrs_tx(proc); if (rc != 0) { @@ -2957,16 +2979,16 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle, done: if (rc != 0) { -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif STATS_INC(ble_gattc_stats, disc_all_chrs_fail); } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (proc != NULL) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -3026,7 +3048,9 @@ ble_gattc_disc_chr_uuid_tx(struct ble_gattc_proc *proc) ble_uuid16_t uuid = BLE_UUID16_INIT(BLE_ATT_UUID_CHARACTERISTIC); int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif rc = ble_att_clt_tx_read_type(proc->conn_handle, proc->cid, proc->disc_chr_uuid.prev_handle + 1, @@ -3186,7 +3210,7 @@ ble_gattc_disc_chrs_by_uuid(uint16_t conn_handle, uint16_t start_handle, struct ble_gattc_proc *proc; int rc; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - bool in_temp_list = false; + int preempt_inserted = 0; #endif #if MYNEWT_VAL(BLE_GATT_CACHING) @@ -3215,10 +3239,8 @@ ble_gattc_disc_chrs_by_uuid(uint16_t conn_handle, uint16_t start_handle, proc->disc_chr_uuid.cb_arg = cb_arg; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - in_temp_list = true; - ble_hs_unlock(); + ble_gattc_proc_preempt_insert(proc); + preempt_inserted = 1; #endif ble_gattc_log_disc_chr_uuid(proc); @@ -3230,15 +3252,15 @@ ble_gattc_disc_chrs_by_uuid(uint16_t conn_handle, uint16_t start_handle, done: if (rc != 0) { STATS_INC(ble_gattc_stats, disc_chrs_uuid_fail); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL && in_temp_list) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (preempt_inserted) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -3297,7 +3319,9 @@ ble_gattc_disc_all_dscs_tx(struct ble_gattc_proc *proc) { int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif rc = ble_att_clt_tx_find_info(proc->conn_handle, proc->cid, proc->disc_all_dscs.prev_handle + 1, @@ -3447,12 +3471,10 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle, proc->disc_all_dscs.cb = cb; proc->disc_all_dscs.cb_arg = cb_arg; -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - ble_hs_unlock(); -#endif ble_gattc_log_disc_all_dscs(proc); +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + ble_gattc_proc_preempt_insert(proc); +#endif rc = ble_gattc_disc_all_dscs_tx(proc); if (rc != 0) { @@ -3461,19 +3483,16 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle, done: if (rc != 0) { - /* One ble_gattc_proc cannot be part of multiple linked lists. - * Hence it needs to be removed before going into ble_gattc_process_status. - */ -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif STATS_INC(ble_gattc_stats, disc_all_dscs_fail); } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (proc != NULL) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -4194,7 +4213,7 @@ ble_gattc_read_by_uuid(uint16_t conn_handle, uint16_t start_handle, struct ble_gattc_proc *proc; int rc; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - bool in_temp_list = false; + int preempt_inserted = 0; #endif STATS_INC(ble_gattc_stats, read_uuid); @@ -4217,10 +4236,8 @@ ble_gattc_read_by_uuid(uint16_t conn_handle, uint16_t start_handle, proc->read_uuid.cb_arg = cb_arg; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - in_temp_list = true; - ble_hs_unlock(); + ble_gattc_proc_preempt_insert(proc); + preempt_inserted = 1; #endif ble_gattc_log_read_uuid(start_handle, end_handle, uuid); rc = ble_gattc_read_uuid_tx(proc); @@ -4231,15 +4248,15 @@ ble_gattc_read_by_uuid(uint16_t conn_handle, uint16_t start_handle, done: if (rc != 0) { STATS_INC(ble_gattc_stats, read_uuid_fail); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL && in_temp_list) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (preempt_inserted) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -4302,7 +4319,9 @@ ble_gattc_read_long_tx(struct ble_gattc_proc *proc) { int rc; +#if !MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) ble_gattc_dbg_assert_proc_not_inserted(proc); +#endif if (proc->read_long.offset == 0) { rc = ble_att_clt_tx_read(proc->conn_handle, proc->cid, proc->read_long.handle); @@ -4430,9 +4449,7 @@ ble_gattc_read_long(uint16_t conn_handle, uint16_t handle, uint16_t offset, proc->read_long.cb_arg = cb_arg; #if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - ble_hs_lock(); - STAILQ_INSERT_TAIL(&temp_proc_list, proc, next); - ble_hs_unlock(); + ble_gattc_proc_preempt_insert(proc); #endif ble_gattc_log_read_long(proc); @@ -4444,15 +4461,15 @@ ble_gattc_read_long(uint16_t conn_handle, uint16_t handle, uint16_t offset, done: if (rc != 0) { STATS_INC(ble_gattc_stats, read_long_fail); -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - if (proc != NULL) { - if (!ble_gattc_proc_remove_preempted(proc)) { - proc = NULL; - } - } -#endif } +#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) + if (proc != NULL) { + ble_gattc_proc_preempt_complete(proc, rc); + return rc; + } +#endif + ble_gattc_process_status(proc, rc, false); return rc; } @@ -7099,9 +7116,6 @@ ble_gattc_init(void) } #endif -#if MYNEWT_VAL(BLE_GATTC_PROC_PREEMPTION_PROTECT) - STAILQ_INIT(&temp_proc_list); -#endif STAILQ_INIT(&ble_gattc_procs); #if MYNEWT_VAL(BLE_GATTC_AUTO_PAIR) STAILQ_INIT(&ble_gattc_cached_procs);