Re: [dpdk-dev] [PATCH v6] app/pdump: add pudmp exits with primary support

[Suanming.Mou <mousuanming@huawei.com>]

On 2019/5/2 16:04, Varghese, Vipin wrote:
> Hi Suanming,
>
> snipped
>>   /* true if x is a power of 2 */
>>   #define POWEROF2(x) ((((x)-1) & (x)) == 0) @@ -413,6 +416,18 @@ struct
> Can we use ` RTE_IS_POWER_OF_2(n) ' instead of ` POWEROF2`?

I'm sorry, but that line is not add by this patch this time.

Maybe another commit is more suitable to fix the previous code.

>
>> parse_val {  }
>>
>>   static void
>> +monitor_primary(void *arg __rte_unused) {
>> +	if (quit_signal)
>> +		return;
>> +
>> +	if (rte_eal_primary_proc_alive(NULL))
>> +		rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary,
>> NULL);
>> +	else
>> +		quit_signal = 1;
>> +}
> This is suggestion, why not omit else part with
>
> `
> if (rte_eal_primary_proc_alive(NULL)) {
> 	rte_eal_alarm_set(MONITOR_INTERVAL, monitor_primary,NULL);
> 	return;
> }
> `
Thanks for the suggestion. It's OK for me. If there's one more vote, I 
will do it.
>
> Snipped
>
> As suggested in v4 can you update the `pdump.rst` on the new behaviour?

As noted in the cover letter in v5. The `exit with primary` 
configuration should be made as default or not is still not confirmed 
from the maintainer.

Since `exit with primary` is now removed in the patch and made as 
default per Anatoly's suggestion and not get more information from the 
maintainer, the update of the doc is also got hung up.

>
> Thanks
> Vipin Varghese
>
>