From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id BFDD92BCE for ; Thu, 2 May 2019 11:12:58 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 May 2019 02:12:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,421,1549958400"; d="scan'208";a="140619853" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.113]) ([10.237.220.113]) by orsmga006.jf.intel.com with ESMTP; 02 May 2019 02:12:55 -0700 To: "Suanming.Mou" , "Varghese, Vipin" , "dev@dpdk.org" References: <1556624124-54930-2-git-send-email-mousuanming@huawei.com> <1556774432-59147-1-git-send-email-mousuanming@huawei.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D340270@BGSMSX101.gar.corp.intel.com> <06d176dd-19f7-4aec-fe5f-8c7018359c07@huawei.com> From: "Burakov, Anatoly" Message-ID: Date: Thu, 2 May 2019 10:12:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <06d176dd-19f7-4aec-fe5f-8c7018359c07@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v6] app/pdump: add pudmp exits with primary support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 May 2019 09:12:59 -0000 On 02-May-19 9:32 AM, Suanming.Mou wrote: > > 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. Yes, if there are issues with the code that aren't directly related to the patch and aren't touched by it, they should be addressed as a separate patch. > >> >>> 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. No preference. Either way works, so i'd keep it as is. -- Thanks, Anatoly From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 8B6BDA0AC5 for ; Thu, 2 May 2019 11:13:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 47DD02BD8; Thu, 2 May 2019 11:13:01 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id BFDD92BCE for ; Thu, 2 May 2019 11:12:58 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 May 2019 02:12:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,421,1549958400"; d="scan'208";a="140619853" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.113]) ([10.237.220.113]) by orsmga006.jf.intel.com with ESMTP; 02 May 2019 02:12:55 -0700 To: "Suanming.Mou" , "Varghese, Vipin" , "dev@dpdk.org" References: <1556624124-54930-2-git-send-email-mousuanming@huawei.com> <1556774432-59147-1-git-send-email-mousuanming@huawei.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D340270@BGSMSX101.gar.corp.intel.com> <06d176dd-19f7-4aec-fe5f-8c7018359c07@huawei.com> From: "Burakov, Anatoly" Message-ID: Date: Thu, 2 May 2019 10:12:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <06d176dd-19f7-4aec-fe5f-8c7018359c07@huawei.com> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v6] app/pdump: add pudmp exits with primary support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190502091255.gID7hxIuCfm2xEZ8xS_gMD9a0Zzy_SIb_Ij296RltN0@z> On 02-May-19 9:32 AM, Suanming.Mou wrote: > > 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. Yes, if there are issues with the code that aren't directly related to the patch and aren't touched by it, they should be addressed as a separate patch. > >> >>> 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. No preference. Either way works, so i'd keep it as is. -- Thanks, Anatoly