From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Guo, Jia" <jia.guo@intel.com>
Subject: Re: [PATCH V16 3/4] eal/linux: uevent parse and process
Date: Thu, 29 Mar 2018 23:03:03 +0800
Message-ID: <b89f50ab-893c-a6b0-63a3-415621527acc@intel.com>
References: <1521610066-12966-3-git-send-email-jia.guo@intel.com>
 <1522063256-3997-1-git-send-email-jia.guo@intel.com>
 <1522063256-3997-4-git-send-email-jia.guo@intel.com>
 <b51b6a3e-4546-9225-481e-ed002b4c2601@intel.com>
 <E923DB57A917B54B9182A2E928D00FA65E018F8D@IRSMSX101.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "jblunck@infradead.org" <jblunck@infradead.org>,
 "shreyansh.jain@nxp.com" <shreyansh.jain@nxp.com>,
 "dev@dpdk.org" <dev@dpdk.org>, "Zhang, Helin" <helin.zhang@intel.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>,
 "Tan, Jianfeng" <jianfeng.tan@intel.com>,
 "stephen@networkplumber.org" <stephen@networkplumber.org>,
 "Richardson, Bruce" <bruce.richardson@intel.com>,
 "Yigit, Ferruh" <ferruh.yigit@intel.com>,
 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
 "gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>,
 "Wu, Jingjing" <jingjing.wu@intel.com>,
 "thomas@monjalon.net" <thomas@monjalon.net>,
 "motih@mellanox.com" <motih@mellanox.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id 36A492C12
 for <dev@dpdk.org>; Thu, 29 Mar 2018 17:03:09 +0200 (CEST)
In-Reply-To: <E923DB57A917B54B9182A2E928D00FA65E018F8D@IRSMSX101.ger.corp.intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

hi, harry

thanks for your review.
On 3/29/2018 9:32 PM, Van Haaren, Harry wrote:
> Two additional input along with Jianfeng's existing comments;
>
>> -----Original Message-----
>> From: Tan, Jianfeng
>> Sent: Wednesday, March 28, 2018 5:16 PM
>> To: Guo, Jia <jia.guo@intel.com>; stephen@networkplumber.org; Richardson,
>> Bruce <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>> Ananyev, Konstantin <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com;
>> Wu, Jingjing <jingjing.wu@intel.com>; thomas@monjalon.net;
>> motih@mellanox.com; Van Haaren, Harry <harry.van.haaren@intel.com>
>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org; Zhang,
>> Helin <helin.zhang@intel.com>
>> Subject: Re: [PATCH V16 3/4] eal/linux: uevent parse and process
>>
>> BTW, adding new .c file needs to update meson.build now.
>>
>> On 3/26/2018 7:20 PM, Jeff Guo wrote:
>>> In order to handle the uevent which have been detected from the kernel
>>> side, add uevent parse and process function to translate the uevent into
>>> device event, which user has subscribe to monitor.
>>>
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>> ---
>>> 1.move all linux specific together
>>> ---
>>>    lib/librte_eal/linuxapp/eal/eal_dev.c | 214
>> +++++++++++++++++++++++++++++++++-
>>>    1 file changed, 211 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c
>> b/lib/librte_eal/linuxapp/eal/eal_dev.c
>>
>>> +static bool monitor_not_started = true;
> This variable should be named "monitor_started", as it is a static var it will be zero by default,
> and the following code is easier to read:
>
> if ( !not_started )   becomes    if (started)
>
make sense.
>
>>>    int __rte_experimental
>>>    rte_dev_event_monitor_start(void)
>>>    {
>>> -	/* TODO: start uevent monitor for linux */
>>> +	int ret;
>>> +
>>> +	if (!monitor_not_started)
>>> +		return 0;
>>> +
>>> +	intr_handle.fd = dev_uev_monitor_fd_new();
>>> +	intr_handle.type = RTE_INTR_HANDLE_DEV_EVENT;
> dev_uev_monitor_fd_new() can return -1 on error, we should check for that case here.
>
you are right.