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 C9828A0A05; Wed, 20 Jan 2021 12:05:08 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 84F77140CD7; Wed, 20 Jan 2021 12:05:08 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id E344B140CC1 for ; Wed, 20 Jan 2021 12:05:06 +0100 (CET) IronPort-SDR: XEG0pfUpzm4SOBesV5MgZX3N3xTgY9JI3/PyipGXt/sK6YPC3Hwt34EXGn+RY78BvDZ40V+6PZ 7wJUxWoKyb8A== X-IronPort-AV: E=McAfee;i="6000,8403,9869"; a="166749035" X-IronPort-AV: E=Sophos;i="5.79,361,1602572400"; d="scan'208";a="166749035" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2021 03:05:05 -0800 IronPort-SDR: rFPgNDjds9ND7Go1TC4Y9ftkJS+/GGD3kR4OT7m7s9v5B7KmOA8Fn7NnzxQQaDyJ6wsdCZdM4q 1suSWoRiRmWg== X-IronPort-AV: E=Sophos;i="5.79,361,1602572400"; d="scan'208";a="351006756" 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:05:03 -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 References: <13958212.9hUD3U9ut9@thomas> <4a74146d-8a9c-a616-d798-198390752d2d@intel.com> <8525653.OzbC6iP21P@thomas> From: "Burakov, Anatoly" Message-ID: Date: Wed, 20 Jan 2021 11:05:00 +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: <8525653.OzbC6iP21P@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 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. -- Thanks, Anatoly