From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 79DF6A0A05; Wed, 20 Jan 2021 12:17:56 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3AD07140CF7; Wed, 20 Jan 2021 12:17:56 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id D30F4140CEF for ; Wed, 20 Jan 2021 12:17:54 +0100 (CET) IronPort-SDR: 7FsioKT43Bz3/ydPChBEPuzp5BGXsrITWIMebZjvatall/WdN7TwDNMz4Mvv+PBQS57+U9ertw bxxdX5oA1rxw== X-IronPort-AV: E=McAfee;i="6000,8403,9869"; a="179234984" X-IronPort-AV: E=Sophos;i="5.79,361,1602572400"; d="scan'208";a="179234984" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 03:17:53 -0800 IronPort-SDR: Z+ZJuXuTZXpeUlXX6bHwkb7/XU6lYwDgMQ8hYxhEIlIHC5CR9n1FxnNYR0JNnaxg1W636rhioe Hc9ilVaxvJcw== X-IronPort-AV: E=Sophos;i="5.79,361,1602572400"; d="scan'208";a="351010961" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.240.112]) ([10.213.240.112]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 03:17:50 -0800 To: Thomas Monjalon Cc: dev@dpdk.org, Timothy McDaniel , Jan Viktorin , Ruifeng Wang , Jerin Jacob , David Christensen , Bruce Richardson , Konstantin Ananyev , david.hunt@intel.com, chris.macnamara@intel.com, david.marchand@redhat.com, ajit.khaparde@broadcom.com, honnappa.nagarahalli@arm.com References: <8525653.OzbC6iP21P@thomas> <3421789.b6iXB7q6zG@thomas> From: "Burakov, Anatoly" Message-ID: Date: Wed, 20 Jan 2021 11:17:48 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <3421789.b6iXB7q6zG@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v17 03/11] eal: change API of power intrinsics X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" On 20-Jan-21 11:11 AM, Thomas Monjalon wrote: > 20/01/2021 12:05, Burakov, Anatoly: >> On 20-Jan-21 10:38 AM, Thomas Monjalon wrote: >>> 20/01/2021 11:32, Burakov, Anatoly: >>>> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote: >>>>> 19/01/2021 12:23, Burakov, Anatoly: >>>>>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote: >>>>>>> 19/01/2021 11:29, Burakov, Anatoly: >>>>>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote: >>>>>>>>> 14/01/2021 15:46, Anatoly Burakov: >>>>>>>>>> +struct rte_power_monitor_cond { >>>>>>>>>> + volatile void *addr; /**< Address to monitor for changes */ >>>>>>>>>> + uint64_t val; /**< Before attempting the monitoring, the address >>>>>>>>>> + * may be read and compared against this value. >>>>>>>>> >>>>>>>>> "may" be read and compared? >>>>>>>>> Is there a case where there is no read and compare? >>>>>>>> >>>>>>>> Yes, if the mask is not set. >>>>>>> >>>>>>> If the mask is not set, the address is "read" anyway >>>>>>> or it is only "watched" for any change? >>>>>>> >>>>>>> Sorry the mechanism is really not clear to me. >>>>>>> >>>>>> >>>>>> The "value" is only used to avoid the sleep, i.e. to check if the write >>>>>> has already happened. We're waiting on *a write* rather than *a value*, >>>>>> so it's not equivalent to "wait until equal" call. It's more of a "sleep >>>>>> until something happens". >>>>> >>>>> Please make things explicit in doxygen. >>>>> The behaviour of each case should be explained crystal clear. >>>>> Thanks >>>> >>>> It is explained in the comments to `rte_power_monitor()` call. But OK, >>>> i'll add more clarification for the struct too. >>> >>> Please avoid the word "may" in API description. >>> >>> This is what is explained in rte_power_monitor: >>> " >>> * Additionally, an `expected` 64-bit value and 64-bit mask are provided. If >>> * mask is non-zero, the current value pointed to by the `p` pointer will be >>> * checked against the expected value, and if they match, the entering of >>> * optimized power state may be aborted. >>> " >>> >>> Can we replace "may" by "will"? >>> >> >> Yep, we can. However, the "may" part was intended to leave some wiggle >> room for a different implementation, should the need arise, and i find >> "will" to be needlessly prescriptive. Frankly, i do not see the need for >> such a detailed description of what the API does under the hood, as long >> as it's clear what its effects are. The main purpose is waiting for a >> write. The mask is only used to check whether the expected write has >> already happened by the time we're calling the API. Whether the CPU then >> does or does not go to sleep is not really relevant IMO. > > I think it is relevant but I may be wrong. > Any other opinions? > I have no objection in documenting that further, so i'll go ahead and do it :) It's just that i think it's not necessary to be *this* detailed about how the API does things. -- Thanks, Anatoly