| Message ID | 1436378469-20444-1-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/08/15 21:01, 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> > --- > v5: fix array use in last hunk > > v6: handle negative 'ret' in last hunk > > test/performance/odp_pktio_perf.c | 15 ++++++++++++++- > test/performance/odp_scheduling.c | 10 ++++++++-- > test/validation/pktio/pktio.c | 5 +++++ > test/validation/queue/queue.c | 4 ++++ > 4 files changed, 31 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; > + still not clear why you use cnt if there is 'int i' and you can reuse it. > 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); with for() 4 lines above will be 2 lines. > + 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..d598dbf 100644 > --- a/test/validation/queue/queue.c > +++ b/test/validation/queue/queue.c > @@ -91,6 +91,10 @@ static void queue_test_sunnydays(void) > */ > ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); > CU_ASSERT(MAX_BUFFER_QUEUE == ret); > + i = ret < 0 ? 0 : ret; > + for ( ; i < MAX_BUFFER_QUEUE; i++) > + odp_event_free(enev[i]); > + > pev_tmp = deev; > do { > deq_ret = odp_queue_deq_multi(queue_id, pev_tmp,
On 09/07/15 10:20, Maxim Uvarov wrote: > On 07/08/15 21:01, 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> >> --- >> v5: fix array use in last hunk >> >> v6: handle negative 'ret' in last hunk >> >> test/performance/odp_pktio_perf.c | 15 ++++++++++++++- >> test/performance/odp_scheduling.c | 10 ++++++++-- >> test/validation/pktio/pktio.c | 5 +++++ >> test/validation/queue/queue.c | 4 ++++ >> 4 files changed, 31 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; >> + > > still not clear why you use cnt if there is 'int i' and you can reuse it. There is no 'int i' here, these are the only local variables. But I've refactored all the hunks to be similar to the last one. >> 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); > with for() 4 lines above will be 2 lines. >> + 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..d598dbf 100644 >> --- a/test/validation/queue/queue.c >> +++ b/test/validation/queue/queue.c >> @@ -91,6 +91,10 @@ static void queue_test_sunnydays(void) >> */ >> ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); >> CU_ASSERT(MAX_BUFFER_QUEUE == ret); >> + i = ret < 0 ? 0 : ret; >> + for ( ; i < MAX_BUFFER_QUEUE; i++) >> + odp_event_free(enev[i]); >> + >> pev_tmp = deev; >> do { >> deq_ret = odp_queue_deq_multi(queue_id, pev_tmp, >
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..d598dbf 100644 --- a/test/validation/queue/queue.c +++ b/test/validation/queue/queue.c @@ -91,6 +91,10 @@ static void queue_test_sunnydays(void) */ ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); CU_ASSERT(MAX_BUFFER_QUEUE == ret); + i = ret < 0 ? 0 : ret; + for ( ; i < MAX_BUFFER_QUEUE; i++) + odp_event_free(enev[i]); + 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> --- v5: fix array use in last hunk v6: handle negative 'ret' in last hunk test/performance/odp_pktio_perf.c | 15 ++++++++++++++- test/performance/odp_scheduling.c | 10 ++++++++-- test/validation/pktio/pktio.c | 5 +++++ test/validation/queue/queue.c | 4 ++++ 4 files changed, 31 insertions(+), 3 deletions(-)