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
