| Message ID | 1435770379-30000-10-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
#0 0x0000000000403632 in _odp_buffer_event_type (buf=0x7fff45022170) at ./include/odp_buffer_inlines.h:195 195 return odp_buf_to_hdr(buf)->event_type; (gdb) bt #0 0x0000000000403632 in _odp_buffer_event_type (buf=0x7fff45022170) at ./include/odp_buffer_inlines.h:195 #1 0x000000000040365b in odp_event_type (event=0x7fff45022170) at odp_event.c:19 #2 0x0000000000403676 in odp_event_free (event=0x7fff45022170) at odp_event.c:24 #3 0x00000000004022dc in queue_test_sunnydays () at queue.c:97 #4 0x00002afb91248482 in run_single_test () from /usr/local/lib/libcunit.so.1 #5 0x00002afb912480b2 in run_single_suite () from /usr/local/lib/libcunit.so.1 #6 0x00002afb91245d55 in CU_run_all_tests () from /usr/local/lib/libcunit.so.1 #7 0x00002afb9124a245 in basic_run_all_tests () from /usr/local/lib/libcunit.so.1 #8 0x00002afb91249fe7 in CU_basic_run_tests () from /usr/local/lib/libcunit.so.1 #9 0x000000000040265d in odp_cunit_run (testsuites=0x6202e0 <queue_suites>) at odp_cunit_common.c:111 #10 0x0000000000402453 in queue_main () at queue.c:132 #11 0x0000000000401f06 in main () at queue_main.c:11 (gdb) p buf $1 = (odp_buffer_t) 0x7fff45022170 (gdb) On 07/01/15 20:06, Zoltan Kiss wrote: > Unsent packet has to be released. If the event type is obvious from the > context, use directly the relevant release functions, otherwise > odp_event(free). > Wider error handling is attempted, but this patch can't fix all the flaws > in the many calling functions of odp_queue_enq() > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > test/performance/odp_pktio_perf.c | 15 ++++++++++++++- > test/performance/odp_scheduling.c | 10 ++++++++-- > test/validation/pktio/pktio.c | 5 +++++ > test/validation/queue/queue.c | 6 ++++++ > 4 files changed, 33 insertions(+), 3 deletions(-) > > diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c > index 52bddc4..6e07c9f 100644 > --- a/test/performance/odp_pktio_perf.c > +++ b/test/performance/odp_pktio_perf.c > @@ -270,6 +270,9 @@ static int alloc_packets(odp_event_t *event_tbl, int num_pkts) > static int send_packets(odp_queue_t outq, > odp_event_t *event_tbl, unsigned num_pkts) > { > + int ret; > + unsigned cnt; > + > if (num_pkts == 0) > return 0; > else if (num_pkts == 1) { > @@ -281,7 +284,17 @@ static int send_packets(odp_queue_t outq, > } > } > > - return odp_queue_enq_multi(outq, event_tbl, num_pkts); > + ret = odp_queue_enq_multi(outq, event_tbl, num_pkts); > + if (ret == (signed)num_pkts) > + return ret; > + > + if (ret < 0) > + ret = 0; > + cnt = ret; > + do > + odp_event_free(event_tbl[cnt]); > + while (++cnt < num_pkts); > + return ret; > } > > /* > diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c > index 1283986..8b46eb2 100644 > --- a/test/performance/odp_scheduling.c > +++ b/test/performance/odp_scheduling.c > @@ -535,9 +535,15 @@ static int test_schedule_multi(const char *str, int thr, > } > > /* Assume we can enqueue all events */ > - if (odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX) != > - MULTI_BUFS_MAX) { > + j = odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX); > + if (j != MULTI_BUFS_MAX) { > LOG_ERR(" [%i] Queue enqueue failed.\n", thr); > + if (j < 0) > + j = 0; > + do > + odp_event_free(ev[j]); > + while (++j < MULTI_BUFS_MAX); > + > return -1; > } > } > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 9d5af22..ac12759 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -389,6 +389,11 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, > ret = odp_queue_enq_multi(pktio_a->outq, tx_ev, num_pkts); > if (ret != num_pkts) { > CU_FAIL("failed to enqueue test packets"); > + if (ret < 0) > + ret = 0; > + do > + odp_packet_free(tx_pkt[ret]); > + while (++ret < num_pkts); > return; > } > } > diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c > index 5f05e49..f78d4b5 100644 > --- a/test/validation/queue/queue.c > +++ b/test/validation/queue/queue.c > @@ -91,6 +91,12 @@ static void queue_test_sunnydays(void) > */ > ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); > CU_ASSERT(MAX_BUFFER_QUEUE == ret); > + if (ret < 0) > + ret = 0; > + do > + odp_event_free(enev[ret]); > + while (++ret < MAX_BUFFER_QUEUE); > + here you run out of array. Also please do not use return value as look iterator. Code might be simple: ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); CU_ASSERT(MAX_BUFFER_QUEUE == ret); for (i = ret; i < MAX_BUFFER_QUEUE; i++) odp_event_free(enev[i]); Thanks, Maxim. > pev_tmp = deev; > do { > deq_ret = odp_queue_deq_multi(queue_id, pev_tmp,
I've fixed it up and sent the patch again (sorry, I've missed the API-NEXT tag from the header, but it is a reply to the original patch On 06/07/15 11:48, Maxim Uvarov wrote: >> > here you run out of array. Also please do not use return value as look > iterator. Code might be simple: > > ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); > CU_ASSERT(MAX_BUFFER_QUEUE == ret); > for (i = ret; i < MAX_BUFFER_QUEUE; i++) > odp_event_free(enev[i]); > > Thanks, > Maxim.
diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c index 52bddc4..6e07c9f 100644 --- a/test/performance/odp_pktio_perf.c +++ b/test/performance/odp_pktio_perf.c @@ -270,6 +270,9 @@ static int alloc_packets(odp_event_t *event_tbl, int num_pkts) static int send_packets(odp_queue_t outq, odp_event_t *event_tbl, unsigned num_pkts) { + int ret; + unsigned cnt; + if (num_pkts == 0) return 0; else if (num_pkts == 1) { @@ -281,7 +284,17 @@ static int send_packets(odp_queue_t outq, } } - return odp_queue_enq_multi(outq, event_tbl, num_pkts); + ret = odp_queue_enq_multi(outq, event_tbl, num_pkts); + if (ret == (signed)num_pkts) + return ret; + + if (ret < 0) + ret = 0; + cnt = ret; + do + odp_event_free(event_tbl[cnt]); + while (++cnt < num_pkts); + return ret; } /* diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c index 1283986..8b46eb2 100644 --- a/test/performance/odp_scheduling.c +++ b/test/performance/odp_scheduling.c @@ -535,9 +535,15 @@ static int test_schedule_multi(const char *str, int thr, } /* Assume we can enqueue all events */ - if (odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX) != - MULTI_BUFS_MAX) { + j = odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX); + if (j != MULTI_BUFS_MAX) { LOG_ERR(" [%i] Queue enqueue failed.\n", thr); + if (j < 0) + j = 0; + do + odp_event_free(ev[j]); + while (++j < MULTI_BUFS_MAX); + return -1; } } diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 9d5af22..ac12759 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -389,6 +389,11 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, ret = odp_queue_enq_multi(pktio_a->outq, tx_ev, num_pkts); if (ret != num_pkts) { CU_FAIL("failed to enqueue test packets"); + if (ret < 0) + ret = 0; + do + odp_packet_free(tx_pkt[ret]); + while (++ret < num_pkts); return; } } diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c index 5f05e49..f78d4b5 100644 --- a/test/validation/queue/queue.c +++ b/test/validation/queue/queue.c @@ -91,6 +91,12 @@ static void queue_test_sunnydays(void) */ ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); CU_ASSERT(MAX_BUFFER_QUEUE == ret); + if (ret < 0) + ret = 0; + do + odp_event_free(enev[ret]); + while (++ret < MAX_BUFFER_QUEUE); + pev_tmp = deev; do { deq_ret = odp_queue_deq_multi(queue_id, pev_tmp,
Unsent packet has to be released. If the event type is obvious from the context, use directly the relevant release functions, otherwise odp_event(free). Wider error handling is attempted, but this patch can't fix all the flaws in the many calling functions of odp_queue_enq() Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> --- test/performance/odp_pktio_perf.c | 15 ++++++++++++++- test/performance/odp_scheduling.c | 10 ++++++++-- test/validation/pktio/pktio.c | 5 +++++ test/validation/queue/queue.c | 6 ++++++ 4 files changed, 33 insertions(+), 3 deletions(-)