From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id C1BF234F0 for ; Wed, 8 May 2019 10:04:06 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 24D1B20D34; Wed, 8 May 2019 04:04:06 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 08 May 2019 04:04:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=KyeHoJy9z9IqsZPdorD56BSzVjd/EDcQwAO/2Q/NoCk=; b=I65Dtm+zmp15 2Utjpz2cNL750Pr+F0cI94ZP3GEzxYHi3ibbSFb05ZJzFXJkIzub1THFhkQQGmsp DpWu2kfAVQMtPHZOAIGLz8lyX9LqF3wd+a+lxxPpFT2vET+FVgrsR3mF3jRhDj54 Bi4cPVpVFeEyYsoDC36J42EoNaWOELU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=KyeHoJy9z9IqsZPdorD56BSzVjd/EDcQwAO/2Q/No Ck=; b=1VZhNnVg21QMtIDST0kY4lRfj6uuQx3VA4ZlbYrk49bdH6pHTdXdAjMFm Uk7xuLrGbCawsw3A91wqwkt3pIfA82HGf34TCe8DcVVRXa2WB7fMmZhcrzSWkLWQ RdMIPZ81/l4x1jP6Obxd+g5BAWpkg1U2Yroc+GpVWxcPVGER980JM64Sd7uQD9lJ mmwBa8frnkRCkAjxXFeqUqPqhyjWtMxwCPWNItu3PEmx8LxUj/0GkXXs+z8ztE78 OBu9V+HBzLL/Pi0K2g9U9wd4LS35mSC6X1fYEZV2Eg5JATvKrTAfina7gIWrGb82 LWjLvhShx+j37wqb1DRhXivNZsHGg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrkedvgddvjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id DA4FD8005A; Wed, 8 May 2019 04:04:04 -0400 (EDT) From: Thomas Monjalon To: "Suanming. Mou" Cc: dev@dpdk.org, vipin.varghese@intel.com, anatoly.burakov@intel.com Date: Wed, 08 May 2019 10:04:03 +0200 Message-ID: <1677555.97jmHZBzRH@xps> In-Reply-To: <1556862508-61677-1-git-send-email-mousuanming@huawei.com> References: <1556800505-59917-1-git-send-email-mousuanming@huawei.com> <1556862508-61677-1-git-send-email-mousuanming@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v8] 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: Wed, 08 May 2019 08:04:07 -0000 Hi, I try to suggest some rewording below. 03/05/2019 07:48, Suanming. Mou: > +/* Enough to set it to 500ms for exiting. */ > +#define MONITOR_INTERVAL (500 * 1000) What is "enough"? What is "it"? What is the relation between MONITOR_INTERVAL and exiting? [...] > + /* > + * Don't worry about it is primary exit case. The alarm cancel > + * function will take care about that. Ignore the ENOENT case. > + */ I don't understand the comment. Maybe you can explicit what is "it" and "that". It would be probably simpler if you just describe why you cancel this alarm. How is it related to ENOENT? > + ret = rte_eal_alarm_cancel(monitor_primary, NULL); > + if (ret < 0) > + printf("Fail to disable monitor:%d\n", ret); [...] > - /* create mempool, ring and vdevs info */ > + /* create mempool, ring, vdevs info and primary monitor */ I don't see any value to this comment. You may just drop it. > create_mp_ring_vdev(); > enable_pdump(); > + enable_primary_monitor(); 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 3BB3EA0096 for ; Wed, 8 May 2019 10:04:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E2215374C; Wed, 8 May 2019 10:04:07 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id C1BF234F0 for ; Wed, 8 May 2019 10:04:06 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 24D1B20D34; Wed, 8 May 2019 04:04:06 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 08 May 2019 04:04:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=KyeHoJy9z9IqsZPdorD56BSzVjd/EDcQwAO/2Q/NoCk=; b=I65Dtm+zmp15 2Utjpz2cNL750Pr+F0cI94ZP3GEzxYHi3ibbSFb05ZJzFXJkIzub1THFhkQQGmsp DpWu2kfAVQMtPHZOAIGLz8lyX9LqF3wd+a+lxxPpFT2vET+FVgrsR3mF3jRhDj54 Bi4cPVpVFeEyYsoDC36J42EoNaWOELU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=KyeHoJy9z9IqsZPdorD56BSzVjd/EDcQwAO/2Q/No Ck=; b=1VZhNnVg21QMtIDST0kY4lRfj6uuQx3VA4ZlbYrk49bdH6pHTdXdAjMFm Uk7xuLrGbCawsw3A91wqwkt3pIfA82HGf34TCe8DcVVRXa2WB7fMmZhcrzSWkLWQ RdMIPZ81/l4x1jP6Obxd+g5BAWpkg1U2Yroc+GpVWxcPVGER980JM64Sd7uQD9lJ mmwBa8frnkRCkAjxXFeqUqPqhyjWtMxwCPWNItu3PEmx8LxUj/0GkXXs+z8ztE78 OBu9V+HBzLL/Pi0K2g9U9wd4LS35mSC6X1fYEZV2Eg5JATvKrTAfina7gIWrGb82 LWjLvhShx+j37wqb1DRhXivNZsHGg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrkedvgddvjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id DA4FD8005A; Wed, 8 May 2019 04:04:04 -0400 (EDT) From: Thomas Monjalon To: "Suanming. Mou" Cc: dev@dpdk.org, vipin.varghese@intel.com, anatoly.burakov@intel.com Date: Wed, 08 May 2019 10:04:03 +0200 Message-ID: <1677555.97jmHZBzRH@xps> In-Reply-To: <1556862508-61677-1-git-send-email-mousuanming@huawei.com> References: <1556800505-59917-1-git-send-email-mousuanming@huawei.com> <1556862508-61677-1-git-send-email-mousuanming@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v8] 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: <20190508080403.OyGSwOvXtg3GLh6XJhxNyeFZBvJqHG56tm6yTQXzswI@z> Hi, I try to suggest some rewording below. 03/05/2019 07:48, Suanming. Mou: > +/* Enough to set it to 500ms for exiting. */ > +#define MONITOR_INTERVAL (500 * 1000) What is "enough"? What is "it"? What is the relation between MONITOR_INTERVAL and exiting? [...] > + /* > + * Don't worry about it is primary exit case. The alarm cancel > + * function will take care about that. Ignore the ENOENT case. > + */ I don't understand the comment. Maybe you can explicit what is "it" and "that". It would be probably simpler if you just describe why you cancel this alarm. How is it related to ENOENT? > + ret = rte_eal_alarm_cancel(monitor_primary, NULL); > + if (ret < 0) > + printf("Fail to disable monitor:%d\n", ret); [...] > - /* create mempool, ring and vdevs info */ > + /* create mempool, ring, vdevs info and primary monitor */ I don't see any value to this comment. You may just drop it. > create_mp_ring_vdev(); > enable_pdump(); > + enable_primary_monitor();