[2/4] linux-dpdk: pool: make sure we have at least one worker for this calculation

Message ID 1450285590-25226-3-git-send-email-zoltan.kiss@linaro.org
State New
Headers show

Commit Message

Zoltan Kiss Dec. 16, 2015, 5:06 p.m.
Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---
 platform/linux-dpdk/odp_pool.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Maxim Uvarov Dec. 16, 2015, 7:46 p.m. | #1
On 12/16/2015 20:06, Zoltan Kiss wrote:
> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> ---
>   platform/linux-dpdk/odp_pool.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/platform/linux-dpdk/odp_pool.c b/platform/linux-dpdk/odp_pool.c
> index 2640c6e..40efb8f 100644
> --- a/platform/linux-dpdk/odp_pool.c
> +++ b/platform/linux-dpdk/odp_pool.c
> @@ -325,6 +325,8 @@ odp_pool_t odp_pool_create(const char *name, odp_pool_param_t *params)
>   			int num_workers =
>   				odp_cpumask_default_worker(&dummy_mask, 0);
>   
> +			if (num_workers < 1)
> +				num_workers = 1;

that hides  wrong output from odp_cpumask_default_worker().

Maxim.
>   			cache_overhead = cache_size * 1.5 * (num_workers - 1);
>   			if (num + cache_overhead < num)
>   				num = UINT32_MAX;
Zoltan Kiss Dec. 17, 2015, 6:44 p.m. | #2
On 16/12/15 19:46, Maxim Uvarov wrote:
> On 12/16/2015 20:06, Zoltan Kiss wrote:
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>>   platform/linux-dpdk/odp_pool.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/platform/linux-dpdk/odp_pool.c
>> b/platform/linux-dpdk/odp_pool.c
>> index 2640c6e..40efb8f 100644
>> --- a/platform/linux-dpdk/odp_pool.c
>> +++ b/platform/linux-dpdk/odp_pool.c
>> @@ -325,6 +325,8 @@ odp_pool_t odp_pool_create(const char *name,
>> odp_pool_param_t *params)
>>               int num_workers =
>>                   odp_cpumask_default_worker(&dummy_mask, 0);
>> +            if (num_workers < 1)
>> +                num_workers = 1;
>
> that hides  wrong output from odp_cpumask_default_worker().

Well, that function doesn't really have its return value properly 
defined. I mean, it's an int, yet a negative value is not explained. 
Probably you should assume it means an error.
Also, 0 is a reasonable value, and in the next multiplication it would 
provide a bad value. I've reworked this to the following:

if (num_workers < 0)
	ODP_ABORT("Worker number fail! %d\n",
		  num_workers);
if (num_workers)
	num_workers -= 1;

cache_overhead = cache_size * 1.5 * num_workers;

Is it OK? In any way, this piece of code is scheduled to be removed.

Zoli


>
> Maxim.
>>               cache_overhead = cache_size * 1.5 * (num_workers - 1);
>>               if (num + cache_overhead < num)
>>                   num = UINT32_MAX;
>
> _______________________________________________
> lng-odp-dpdk mailing list
> lng-odp-dpdk@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp-dpdk

Patch hide | download patch | download mbox

diff --git a/platform/linux-dpdk/odp_pool.c b/platform/linux-dpdk/odp_pool.c
index 2640c6e..40efb8f 100644
--- a/platform/linux-dpdk/odp_pool.c
+++ b/platform/linux-dpdk/odp_pool.c
@@ -325,6 +325,8 @@  odp_pool_t odp_pool_create(const char *name, odp_pool_param_t *params)
 			int num_workers =
 				odp_cpumask_default_worker(&dummy_mask, 0);
 
+			if (num_workers < 1)
+				num_workers = 1;
 			cache_overhead = cache_size * 1.5 * (num_workers - 1);
 			if (num + cache_overhead < num)
 				num = UINT32_MAX;