app/dma-perf: calrify incorrect NUMA config
Checks
Commit Message
Current commit decalres either `source or destination numa is greater
than acture numa` as cause of error. Rephrase as `Source or Destination`
is incorrect numa by checking which is greater than available numa.
Signed-off-by: Vipin Varghese <vipin.varghese@amd.com>
---
app/test-dma-perf/benchmark.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
Hi Vipin,
On 2024/3/6 23:02, Vipin Varghese wrote:
> Current commit decalres either `source or destination numa is greater
> than acture numa` as cause of error. Rephrase as `Source or Destination`
> is incorrect numa by checking which is greater than available numa.
>
> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com>
> ---
> app/test-dma-perf/benchmark.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
> index 9b1f58c78c..5634ff8bf8 100644
> --- a/app/test-dma-perf/benchmark.c
> +++ b/app/test-dma-perf/benchmark.c
> @@ -313,7 +313,8 @@ setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
> nr_sockets = rte_socket_count();
> if (cfg->src_numa_node >= nr_sockets ||
> cfg->dst_numa_node >= nr_sockets) {
> - printf("Error: Source or destination numa exceeds the acture numa nodes.\n");
> + printf("Error: %s numa exceeds the available numa nodes.\n",
> + (cfg->src_numa_node >= nr_sockets) ? "Source" : "Destination");
There are three cases to invoking:
1) src_numa_node >= nr_sockets
2) dst_numa_node >= nr_sockets
3) both src_numa_node and dst_numa_node >= nr_sockets
It could cover cases 1&2 in your commit, but could not cover case 3.
So I think we should keep original implement.
Thanks
> return -1;
> }
>
>
<snipped>
>> - printf("Error: Source or destination numa exceeds the acture numa nodes.\n");
>> + printf("Error: %s numa exceeds the available numa nodes.\n",
>> + (cfg->src_numa_node >= nr_sockets) ? "Source" : "Destination");
Thank you for comments, please find my reasoning as `change is added to
address spelling issue of acture numa to available numa`
> There are three cases to invoking:
> 1) src_numa_node >= nr_sockets
> 2) dst_numa_node >= nr_sockets
> 3) both src_numa_node and dst_numa_node >= nr_sockets
As per my testing, following scenarios are explored
1) if src-numa is incorrect, we get error message as `source is
incorrect numa`
2) if dst-numa is incorrect, we get error message as `destination is
incorrect numa`
3) if both src-numa and dst-numa are incorrect, we get error as `source
is incorrect numa`, fixing source and rerunning `destination is
incorrect numa` (which is expected)
>
> It could cover cases 1&2 in your commit, but could not cover case 3.
I am happy to make changes to reflect the third scenario also. But
please note as shared, the real intention is to fix ` acture numa` to
something meaningful.
> So I think we should keep original implement.
I humbly disagree, based on the explanation as shared above. I can share
v2 patch to address
1. acture numa
2. soruce or destination or src & destination
<snipped>
@@ -313,7 +313,8 @@ setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
nr_sockets = rte_socket_count();
if (cfg->src_numa_node >= nr_sockets ||
cfg->dst_numa_node >= nr_sockets) {
- printf("Error: Source or destination numa exceeds the acture numa nodes.\n");
+ printf("Error: %s numa exceeds the available numa nodes.\n",
+ (cfg->src_numa_node >= nr_sockets) ? "Source" : "Destination");
return -1;
}