Skip to content

Commit

Permalink
Bluetooth: controller: nRF5: Check preempt event on timeout
Browse files Browse the repository at this point in the history 
Check whether the preempt event matches with the head of the
pipeline before aborting the currently active event.

This is required to avoid preemption of events that became
active due to done event and there has been a race in
stopping the preempt ticker.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
  • Loading branch information
@cvinayak @carlescufi
cvinayak authored and carlescufi committed May 6, 2021
1 parent 6916eb9 commit 5b75bdf
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 41 deletions.
10 changes: 5 additions & 5 deletions subsys/bluetooth/controller/ll_sw/lll.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,11 @@ int lll_csrand_isr_get(void *buf, size_t len);
int lll_rand_get(void *buf, size_t len);
int lll_rand_isr_get(void *buf, size_t len);

int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
lll_abort_cb_t abort_cb,
struct lll_prepare_param *prepare_param,
lll_prepare_cb_t prepare_cb,
uint8_t is_resume);
struct lll_event *ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
lll_abort_cb_t abort_cb,
struct lll_prepare_param *prepare_param,
lll_prepare_cb_t prepare_cb,
uint8_t is_resume);
void *ull_prepare_dequeue_get(void);
void *ull_prepare_dequeue_iter(uint8_t *idx);
void ull_prepare_dequeue(uint8_t caller_id);
Expand Down
100 changes: 71 additions & 29 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ static int init_reset(void);
#if defined(CONFIG_BT_CTLR_LOW_LAT_ULL_DONE)
static inline void done_inc(void);
#endif /* CONFIG_BT_CTLR_LOW_LAT_ULL_DONE */
static int resume_enqueue(lll_prepare_cb_t resume_cb);
static struct lll_event *resume_enqueue(lll_prepare_cb_t resume_cb);
static void isr_race(void *param);

#if !defined(CONFIG_BT_CTLR_LOW_LAT)
static void ticker_stop_op_cb(uint32_t status, void *param);
static void ticker_start_op_cb(uint32_t status, void *param);
static void preempt_ticker_start(struct lll_prepare_param *prepare_param);
static void ticker_start_next_op_cb(uint32_t status, void *param);
static uint32_t preempt_ticker_start(struct lll_event *event,
ticker_op_func op_cb);
static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
uint16_t lazy, uint8_t force, void *param);
static void preempt(void *param);
Expand Down Expand Up @@ -304,7 +306,6 @@ int lll_done(void *param)
struct lll_event *next;
struct ull_hdr *ull;
void *evdone;
int ret = 0;

/* Assert if param supplied without a pending prepare to cancel. */
next = ull_prepare_dequeue_get();
Expand Down Expand Up @@ -352,7 +353,7 @@ int lll_done(void *param)
evdone = ull_event_done(ull);
LL_ASSERT(evdone);

return ret;
return 0;
}

#if defined(CONFIG_BT_CTLR_LOW_LAT_ULL_DONE)
Expand Down Expand Up @@ -620,25 +621,30 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
(p && is_resume)) {
#if defined(CONFIG_BT_CTLR_LOW_LAT)
lll_prepare_cb_t resume_cb;
struct lll_event *next;
#endif /* CONFIG_BT_CTLR_LOW_LAT */
struct lll_event *next;

if (IS_ENABLED(CONFIG_BT_CTLR_LOW_LAT) && event.curr.param) {
/* early abort */
event.curr.abort_cb(NULL, event.curr.param);
}

/* Store the next prepare for deferred call */
err = ull_prepare_enqueue(is_abort_cb, abort_cb, prepare_param,
prepare_cb, is_resume);
LL_ASSERT(!err);
next = ull_prepare_enqueue(is_abort_cb, abort_cb, prepare_param,
prepare_cb, is_resume);
LL_ASSERT(next);

#if !defined(CONFIG_BT_CTLR_LOW_LAT)
if (is_resume) {
return -EINPROGRESS;
}

preempt_ticker_start(prepare_param);
/* Start the preempt timeout */
uint32_t ret;

ret = preempt_ticker_start(next, ticker_start_op_cb);
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_BUSY));

#else /* CONFIG_BT_CTLR_LOW_LAT */
next = NULL;
Expand All @@ -664,8 +670,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
LL_ASSERT(err);

if (err == -EAGAIN) {
err = resume_enqueue(resume_cb);
LL_ASSERT(!err);
next = resume_enqueue(resume_cb);
LL_ASSERT(next);
} else {
LL_ASSERT(err == -ECANCELED);
}
Expand All @@ -675,6 +681,8 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
return -EINPROGRESS;
}

LL_ASSERT(!p || &p->prepare_param == prepare_param);

event.curr.param = prepare_param->param;
event.curr.is_abort_cb = is_abort_cb;
event.curr.abort_cb = abort_cb;
Expand All @@ -701,13 +709,16 @@ int lll_prepare_resolve(lll_is_abort_cb_t is_abort_cb, lll_abort_cb_t abort_cb,
}
} while (p->is_aborted || p->is_resume);

preempt_ticker_start(&p->prepare_param);
/* Start the preempt timeout */
ret = preempt_ticker_start(p, ticker_start_next_op_cb);
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_BUSY));
#endif /* !CONFIG_BT_CTLR_LOW_LAT */

return err;
}

static int resume_enqueue(lll_prepare_cb_t resume_cb)
static struct lll_event *resume_enqueue(lll_prepare_cb_t resume_cb)
{
struct lll_prepare_param prepare_param = {0};

Expand Down Expand Up @@ -747,16 +758,26 @@ static void ticker_start_op_cb(uint32_t status, void *param)
(status == TICKER_STATUS_FAILURE));
}

static void preempt_ticker_start(struct lll_prepare_param *prepare_param)
static void ticker_start_next_op_cb(uint32_t status, void *param)
{
ARG_UNUSED(param);

LL_ASSERT(status == TICKER_STATUS_SUCCESS);
}

static uint32_t preempt_ticker_start(struct lll_event *event,
ticker_op_func op_cb)
{
struct lll_prepare_param *p;
uint32_t preempt_anchor;
struct ull_hdr *ull;
uint32_t preempt_to;
uint32_t ret;

/* Calc the preempt timeout */
ull = HDR_LLL2ULL(prepare_param->param);
preempt_anchor = prepare_param->ticks_at_expire;
p = &event->prepare_param;
ull = HDR_LLL2ULL(p->param);
preempt_anchor = p->ticks_at_expire;
preempt_to = MAX(ull->ticks_active_to_start,
ull->ticks_prepare_to_start) -
ull->ticks_preempt_to_start;
Expand All @@ -771,11 +792,10 @@ static void preempt_ticker_start(struct lll_prepare_param *prepare_param)
TICKER_NULL_REMAINDER,
TICKER_NULL_LAZY,
TICKER_NULL_SLOT,
preempt_ticker_cb, NULL,
ticker_start_op_cb, NULL);
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_FAILURE) ||
(ret == TICKER_STATUS_BUSY));
preempt_ticker_cb, event,
op_cb, event);

return ret;
}

static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
Expand All @@ -785,6 +805,7 @@ static void preempt_ticker_cb(uint32_t ticks_at_expire, uint32_t remainder,
static struct mayfly mfy = {0, 0, &link, NULL, preempt};
uint32_t ret;

mfy.param = param;
ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH, TICKER_USER_ID_LLL,
0, &mfy);
LL_ASSERT(!ret);
Expand All @@ -795,43 +816,63 @@ static void preempt(void *param)
lll_prepare_cb_t resume_cb;
struct lll_event *next;
uint8_t idx;
int ret;
int err;

/* No event to abort */
if (!event.curr.abort_cb || !event.curr.param) {
return;
}

/* Check if any prepare in pipeline */
idx = UINT8_MAX;
next = ull_prepare_dequeue_iter(&idx);
if (!next) {
return;
}

/* Find a prepare that is ready and not a resume */
while (next && (next->is_aborted || next->is_resume)) {
next = ull_prepare_dequeue_iter(&idx);
}

/* No ready prepare */
if (!next) {
return;
}

ret = event.curr.is_abort_cb(next->prepare_param.param,
/* Preemptor not in pipeline */
if (next != param) {
uint32_t ret;

/* Start the preempt timeout */
ret = preempt_ticker_start(next, ticker_start_next_op_cb);
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
(ret == TICKER_STATUS_BUSY));

return;
}

/* Check if current event want to continue */
err = event.curr.is_abort_cb(next->prepare_param.param,
event.curr.param,
&resume_cb);
if (!ret) {
/* Let LLL know about the cancelled prepare */
if (!err) {
/* Let preemptor LLL know about the cancelled prepare */
next->is_aborted = 1;
next->abort_cb(&next->prepare_param, next->prepare_param.param);

return;
}

/* Abort the current event */
event.curr.abort_cb(NULL, event.curr.param);

if (ret == -EAGAIN) {
/* Check if resume requested */
if (err == -EAGAIN) {
struct lll_event *iter;
uint8_t iter_idx;

/* Abort any duplicates so that they get dequeued */
iter_idx = UINT8_MAX;
iter = ull_prepare_dequeue_iter(&iter_idx);
while (iter) {
Expand All @@ -853,10 +894,11 @@ static void preempt(void *param)
iter = ull_prepare_dequeue_iter(&iter_idx);
}

ret = resume_enqueue(resume_cb);
LL_ASSERT(!ret);
/* Enqueue as resume event */
iter = resume_enqueue(resume_cb);
LL_ASSERT(iter);
} else {
LL_ASSERT(ret == -ECANCELED);
LL_ASSERT(err == -ECANCELED);
}
}
#else /* CONFIG_BT_CTLR_LOW_LAT */
Expand Down
14 changes: 7 additions & 7 deletions subsys/bluetooth/controller/ll_sw/ull.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1637,18 +1637,18 @@ void ull_rx_sched_done(void)
}
#endif /* CONFIG_BT_CTLR_LOW_LAT_ULL */

int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
lll_abort_cb_t abort_cb,
struct lll_prepare_param *prepare_param,
lll_prepare_cb_t prepare_cb,
uint8_t is_resume)
struct lll_event *ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,
lll_abort_cb_t abort_cb,
struct lll_prepare_param *prepare_param,
lll_prepare_cb_t prepare_cb,
uint8_t is_resume)
{
struct lll_event *e;
uint8_t idx;

idx = MFIFO_ENQUEUE_GET(prep, (void **)&e);
if (!e) {
return -ENOBUFS;
return NULL;
}

memcpy(&e->prepare_param, prepare_param, sizeof(e->prepare_param));
Expand All @@ -1660,7 +1660,7 @@ int ull_prepare_enqueue(lll_is_abort_cb_t is_abort_cb,

MFIFO_ENQUEUE(prep, idx);

return 0;
return e;
}

void *ull_prepare_dequeue_get(void)
Expand Down

0 comments on commit 5b75bdf

Please sign in to comment.