From: Tomasz Jozwiak <tjozwiakgm@gmail.com>
To: Shally Verma <shallyv@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>,
"fiona.trahe@intel.com" <fiona.trahe@intel.com>,
"arturx.trybula@intel.com" <arturx.trybula@intel.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH v4 3/6] app/test-compress-perf: add verification test case
Date: Sun, 30 Jun 2019 23:02:52 +0200 [thread overview]
Message-ID: <1b1a4fb3-f44d-2118-4175-c3771aac0d98@gmail.com> (raw)
In-Reply-To: <BN6PR1801MB205272FEE059EB375EC9024BADFE0@BN6PR1801MB2052.namprd18.prod.outlook.com>
Hi Shally,
Thx for the review.
My comments below:
>
>> -----Original Message-----
>> From: Tomasz Jozwiak <tjozwiakgm@gmail.com>
>> Sent: Friday, June 28, 2019 3:56 AM
>> To: dev@dpdk.org; fiona.trahe@intel.com; tjozwiakgm@gmail.com; Shally
>> Verma <shallyv@marvell.com>; arturx.trybula@intel.com
>> Subject: [EXT] [PATCH v4 3/6] app/test-compress-perf: add verification test
>> case
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> From: Tomasz Jozwiak <tomaszx.jozwiak@intel.com>
> ...
>
>> diff --git a/app/test-compress-perf/comp_perf_test_verify.c b/app/test-
>> compress-perf/comp_perf_test_verify.c
>> index 28a0fe8..c2aab70 100644
>> --- a/app/test-compress-perf/comp_perf_test_verify.c
>> +++ b/app/test-compress-perf/comp_perf_test_verify.c
>> @@ -8,14 +8,48 @@
>> #include <rte_compressdev.h>
>>
>> #include "comp_perf_test_verify.h"
>> +#include "comp_perf_test_common.h"
>> +
>> +void
>> +cperf_verify_test_destructor(void *arg) {
>> + if (arg) {
>> + comp_perf_free_memory(&((struct cperf_verify_ctx *)arg)-
>>> mem);
>> + rte_free(arg);
>> + }
>> +}
>> +
>> +void *
>> +cperf_verify_test_constructor(uint8_t dev_id, uint16_t qp_id,
>> + struct comp_test_data *options)
>> +{
>> + struct cperf_verify_ctx *ctx = NULL;
>> +
>> + ctx = rte_malloc(NULL, sizeof(struct cperf_verify_ctx), 0);
>> +
> Better just return from here
[Tomek]. Yes you right. we wasn't able to allocate 'cperf_verify_ctx
struct',
so we don't need to call destructor here. I assume there's the same issue
in benchmark patch - will align both in V5. Thx.
>
>> + if (ctx != NULL) {
>> + ctx->mem.dev_id = dev_id;
>> + ctx->mem.qp_id = qp_id;
>> + ctx->options = options;
>> +
>> + if (!comp_perf_allocate_memory(ctx->options, &ctx->mem)
>> &&
>> + !prepare_bufs(ctx->options, &ctx->mem))
>> + return ctx;
> What if condition fails on comp_per_allocate_memory(), then it will go to verify_test_destructor(), so comp_perf_free_memory() check if mem != NULL before calling actual free?
[Tomek] I mean it's ok. Please take in to account that we was able to
allocate 'cperf_verify_ctx struct' - cause
ctx != NULL here. that means 'mem struct' inside 'cperf_verify_ctx
struct' exists for sure:
struct cperf_verify_ctx {
*struct cperf_mem_resources mem;*
struct comp_test_data *options;
int silent;
size_t comp_data_sz;
size_t decomp_data_sz;
double ratio;
};
and all fields inside 'struct cperf_mem_resources mem' are zeroed.
We don't need to check mem != NULL before free, because in this place
mem != NULL for sure. Also it's ok to call 'rte_free',
'rte_mempool_free' and 'rte_pktmbuf_free' with NULL ptr.
as a argument because the check is inside all of these functions.
Thx for the comments.
--
Tomek
next prev parent reply other threads:[~2019-06-30 21:02 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-30 8:06 [dpdk-dev] [PATCH v1 0/7] add multiple cores feature to test-compress-perf Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 1/7] app/test-compress-perf: add weak functions for multi-cores test Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 2/7] app/test-compress-perf: add ptest command line option Tomasz Jozwiak
2019-06-03 13:35 ` [dpdk-dev] [EXT] " Shally Verma
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 3/7] app/test-compress-perf: add verification test case Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 4/7] app/test-compress-perf: add benchmark " Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 5/7] doc: update dpdk-test-compress-perf description Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 6/7] app/test-compress-perf: add force process termination Tomasz Jozwiak
2019-05-30 8:06 ` [dpdk-dev] [PATCH v1 7/7] doc: update release notes for 19.08 Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 0/7] add multiple cores feature to test-compress-perf Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 1/7] app/test-compress-perf: add weak functions for multi-cores test Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 2/7] app/test-compress-perf: add ptest command line option Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 3/7] app/test-compress-perf: add verification test case Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 4/7] app/test-compress-perf: add benchmark " Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 5/7] doc: update dpdk-test-compress-perf description Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 6/7] app/test-compress-perf: add force process termination Tomasz Jozwiak
2019-06-08 22:22 ` [dpdk-dev] [PATCH v2 7/7] doc: update release notes for 19.08 Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 0/7] add multiple cores feature to test-compress-perf Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 1/7] app/test-compress-perf: add weak functions for multi-cores test Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 2/7] app/test-compress-perf: add ptest command line option Tomasz Jozwiak
2019-06-26 17:13 ` [dpdk-dev] [EXT] " Shally Verma
2019-06-26 17:34 ` Tomasz Jozwiak
2019-06-27 4:41 ` Shally Verma
2019-06-27 21:27 ` Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 3/7] app/test-compress-perf: add verification test case Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 4/7] app/test-compress-perf: add benchmark " Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 5/7] doc: update dpdk-test-compress-perf description Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 6/7] app/test-compress-perf: add force process termination Tomasz Jozwiak
2019-06-26 16:30 ` [dpdk-dev] [PATCH v3 7/7] doc: update release notes for 19.08 Tomasz Jozwiak
2019-06-26 21:26 ` Thomas Monjalon
2019-06-27 21:25 ` Tomasz Jozwiak
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 0/6] add multiple cores feature to test-compress-perf Tomasz Jozwiak
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 1/6] app/test-compress-perf: add weak functions for multi-cores test Tomasz Jozwiak
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 2/6] app/test-compress-perf: add ptest command line option Tomasz Jozwiak
2019-06-30 14:41 ` [dpdk-dev] [EXT] " Shally Verma
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 3/6] app/test-compress-perf: add verification test case Tomasz Jozwiak
2019-06-30 14:55 ` [dpdk-dev] [EXT] " Shally Verma
2019-06-30 21:02 ` Tomasz Jozwiak [this message]
2019-07-01 4:29 ` Shally Verma
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 4/6] app/test-compress-perf: add benchmark " Tomasz Jozwiak
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 5/6] doc: update dpdk-test-compress-perf description Tomasz Jozwiak
2019-06-30 14:56 ` [dpdk-dev] [EXT] " Shally Verma
2019-06-27 22:25 ` [dpdk-dev] [PATCH v4 6/6] app/test-compress-perf: add force process termination Tomasz Jozwiak
2019-06-30 15:00 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 0/6] add multiple cores feature to test-compress-perf Tomasz Jozwiak
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 1/6] app/test-compress-perf: add weak functions for multi-cores test Tomasz Jozwiak
2019-07-02 10:03 ` Trybula, ArturX
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 0/6] add multiple cores feature to test-compress-perf Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 1/6] app/test-compress-perf: add weak functions for multi-cores test Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 2/6] app/test-compress-perf: add ptest command line option Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 3/6] app/test-compress-perf: add verification test case Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 4/6] app/test-compress-perf: add benchmark " Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 5/6] doc: update dpdk-test-compress-perf description Artur Trybula
2019-07-03 15:24 ` [dpdk-dev] [PATCH v6 6/6] app/test-compress-perf: add force process termination Artur Trybula
2019-07-05 9:50 ` [dpdk-dev] [PATCH v6 0/6] add multiple cores feature to test-compress-perf Shally Verma
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 " Fiona Trahe
2019-07-06 9:36 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 1/6] app/test-compress-perf: add weak functions for multi-cores test Fiona Trahe
2019-07-06 9:31 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 0/7] add multiple cores feature to test-compress-perf Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 1/7] app/test-compress-perf: add weak functions for multi-cores test Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 2/7] app/test-compress-perf: add ptest command line option Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 3/7] app/test-compress-perf: add verification test case Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 4/7] app/test-compress-perf: add benchmark " Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 5/7] doc: update dpdk-test-compress-perf description Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 6/7] app/test-compress-perf: add force process termination Artur Trybula
2019-07-08 18:16 ` [dpdk-dev] [PATCH v8 7/7] app/test-compress-perf: 'magic numbers' removed Artur Trybula
2019-07-15 10:03 ` Trahe, Fiona
2019-07-15 13:12 ` [dpdk-dev] [PATCH v8 0/7] add multiple cores feature to test-compress-perf Akhil Goyal
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 2/6] app/test-compress-perf: add ptest command line option Fiona Trahe
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 3/6] app/test-compress-perf: add verification test case Fiona Trahe
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 4/6] app/test-compress-perf: add benchmark " Fiona Trahe
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 5/6] doc: update dpdk-test-compress-perf description Fiona Trahe
2019-07-05 11:15 ` [dpdk-dev] [PATCH v7 6/6] app/test-compress-perf: add force process termination Fiona Trahe
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 2/6] app/test-compress-perf: add ptest command line option Tomasz Jozwiak
2019-07-02 10:05 ` Trybula, ArturX
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 3/6] app/test-compress-perf: add verification test case Tomasz Jozwiak
2019-07-02 10:02 ` Trybula, ArturX
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 4/6] app/test-compress-perf: add benchmark " Tomasz Jozwiak
2019-07-02 10:02 ` Trybula, ArturX
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 5/6] doc: update dpdk-test-compress-perf description Tomasz Jozwiak
2019-07-02 10:04 ` Trybula, ArturX
2019-07-01 11:26 ` [dpdk-dev] [PATCH v5 6/6] app/test-compress-perf: add force process termination Tomasz Jozwiak
2019-07-02 10:02 ` Trybula, ArturX
2019-07-03 10:21 ` [dpdk-dev] [PATCH v5 0/6] add multiple cores feature to test-compress-perf Akhil Goyal
2019-07-03 12:20 ` Tomasz Jóźwiak
[not found] ` <1560031175-13787-1-git-send-email-tjozwiakgm@gmail.com>
2019-06-09 4:53 ` [dpdk-dev] [EXT] [PATCH v2 0/7] " Shally Verma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1b1a4fb3-f44d-2118-4175-c3771aac0d98@gmail.com \
--to=tjozwiakgm@gmail.com \
--cc=arturx.trybula@intel.com \
--cc=dev@dpdk.org \
--cc=fiona.trahe@intel.com \
--cc=shallyv@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).