I've confirmed I can reproduce the failure, but it obviously doesn't happen very often. I'll open a bug for it and investigate further as normal priority.
Thanks. On Fri, Oct 30, 2015 at 6:14 AM, Bill Fischofer <[email protected]> wrote: > Thanks. I believe it should still be merged as it's much more reliable > than before, and the CUnit test is more thorough, which is how we found the > base problem. If there are still issues they should be addressed as > another bug item. > > On Fri, Oct 30, 2015 at 2:53 AM, Maxim Uvarov <[email protected]> > wrote: > >> Bill, >> >> I merged but looks like it was not tested enough. Usually it passes, but >> in some cases it fails: >> 1. scheduler.c:577 - bctx->sequence == seq >> 2. scheduler.c:577 - bctx->sequence == seq >> >> I can reproduce issue with that script: >> while true; do ./test/validation/scheduler/scheduler_main 2>&1 |grep >> FAIL; done >> >> Test: scheduler_test_mq_mt_prio_o ...FAILED >> Test: scheduler_test_mq_mt_prio_o ...FAILED >> Test: scheduler_test_multi_mq_mt_prio_o ...FAILED >> Test: scheduler_test_mq_mt_prio_o ...FAILED >> Test: scheduler_test_mq_mt_prio_o ...FAILED >> Test: scheduler_test_multi_mq_mt_prio_o ...FAILED >> Test: scheduler_test_multi_mq_mt_prio_o ...FAILED >> Test: scheduler_test_multi_mq_mt_prio_o ...FAILED >> Test: scheduler_test_mq_mt_prio_o ...FAILED >> >> Maxim. >> >> >> >> On 10/30/2015 10:14, Maxim Uvarov wrote: >> >>> Merged to api-next. >>> Will merge to master soon. >>> >>> Maxim. >>> >>> On 10/29/2015 05:32, Bill Fischofer wrote: >>> >>>> During odp_schedule_release_ordered() processing in order elements >>>> on a reorder queue should be processed even if they are marked sustain >>>> since no further elements with the current order will be enqueued. >>>> >>>> This fixes Bug https://bugs.linaro.org/show_bug.cgi?id=1824 >>>> >>>> Signed-off-by: Bill Fischofer <[email protected]> >>>> --- >>>> platform/linux-generic/include/odp_queue_internal.h | 5 +++-- >>>> platform/linux-generic/odp_queue.c | 18 >>>> +++++++++++------- >>>> 2 files changed, 14 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/platform/linux-generic/include/odp_queue_internal.h >>>> b/platform/linux-generic/include/odp_queue_internal.h >>>> index c322e6a..6322948 100644 >>>> --- a/platform/linux-generic/include/odp_queue_internal.h >>>> +++ b/platform/linux-generic/include/odp_queue_internal.h >>>> @@ -291,7 +291,8 @@ static inline int reorder_deq(queue_entry_t *queue, >>>> static inline void reorder_complete(queue_entry_t *origin_qe, >>>> odp_buffer_hdr_t **reorder_buf_return, >>>> odp_buffer_hdr_t **placeholder_buf, >>>> - int placeholder_append) >>>> + int placeholder_append, >>>> + int order_released) >>>> { >>>> odp_buffer_hdr_t *reorder_buf = origin_qe->s.reorder_head; >>>> odp_buffer_hdr_t *next_buf; >>>> @@ -311,7 +312,7 @@ static inline void reorder_complete(queue_entry_t >>>> *origin_qe, >>>> reorder_buf = next_buf; >>>> order_release(origin_qe, 1); >>>> - } else if (reorder_buf->flags.sustain) { >>>> + } else if (!order_released && reorder_buf->flags.sustain) { >>>> reorder_buf = next_buf; >>>> } else { >>>> *reorder_buf_return = origin_qe->s.reorder_head; >>>> diff --git a/platform/linux-generic/odp_queue.c >>>> b/platform/linux-generic/odp_queue.c >>>> index a7022cf..a27af0b 100644 >>>> --- a/platform/linux-generic/odp_queue.c >>>> +++ b/platform/linux-generic/odp_queue.c >>>> @@ -478,7 +478,8 @@ int queue_enq(queue_entry_t *queue, >>>> odp_buffer_hdr_t *buf_hdr, int sustain) >>>> * other queues, appending placeholder bufs as needed. >>>> */ >>>> UNLOCK(&queue->s.lock); >>>> - reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, 1); >>>> + reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, >>>> + 1, 0); >>>> UNLOCK(&origin_qe->s.lock); >>>> if (reorder_buf) >>>> @@ -844,7 +845,7 @@ int queue_pktout_enq(queue_entry_t *queue, >>>> odp_buffer_hdr_t *buf_hdr, >>>> order_release(origin_qe, release_count + placeholder_count); >>>> /* Now handle sends to other queues that are ready to go */ >>>> - reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, 1); >>>> + reorder_complete(origin_qe, &reorder_buf, &placeholder_buf, 1, 0); >>>> /* We're fully done with the origin_qe at last */ >>>> UNLOCK(&origin_qe->s.lock); >>>> @@ -921,13 +922,16 @@ int release_order(queue_entry_t *origin_qe, >>>> uint64_t order, >>>> if (order <= origin_qe->s.order_out) { >>>> order_release(origin_qe, 1); >>>> - /* Check if this release allows us to unblock waiters. >>>> - * At the point of this call, the reorder list may contain >>>> - * zero or more placeholders that need to be freed, followed >>>> - * by zero or one complete reorder buffer chain. >>>> + /* Check if this release allows us to unblock waiters. At the >>>> + * point of this call, the reorder list may contain zero or >>>> + * more placeholders that need to be freed, followed by zero >>>> + * or one complete reorder buffer chain. Note that since we >>>> + * are releasing order, we know no further enqs for this order >>>> + * can occur, so ignore the sustain bit to clear out our >>>> + * element(s) on the reorder queue >>>> */ >>>> reorder_complete(origin_qe, &reorder_buf, >>>> - &placeholder_buf_hdr, 0); >>>> + &placeholder_buf_hdr, 0, 1); >>>> /* Now safe to unlock */ >>>> UNLOCK(&origin_qe->s.lock); >>>> >>> >>> >> >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
