From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "Loftus, Ciara" <ciara.loftus@intel.com>,
"Hunt, David" <david.hunt@intel.com>
Subject: Re: [dpdk-dev] [PATCH v1 1/7] power_intrinsics: allow monitor checks inversion
Date: Thu, 24 Jun 2021 16:04:45 +0100 [thread overview]
Message-ID: <882380b2-20b9-f081-9a5a-199c38094b1b@intel.com> (raw)
In-Reply-To: <DM6PR11MB44918C1C1C82EDD5DD3715AB9A079@DM6PR11MB4491.namprd11.prod.outlook.com>
On 24-Jun-21 3:57 PM, Ananyev, Konstantin wrote:
>
>
>>>>>> I did a quick prototype for this, and i don't think it is going to work.
>>>>>>
>>>>>> Callbacks with just "current value" as argument will be pretty limited
>>>>>> and will only really work for cases where we know what we are expecting.
>>>>>> However, for cases like event/dlb or net/mlx5, the expected value is (or
>>>>>> appears to be) dependent upon some internal device data, and is not
>>>>>> constant like in case of net/ixgbe for example.
>>>>>>
>>>>>> This can be fixed by passing an opaque pointer, either by storing it in
>>>>>> the monitor condition, or by passing it directly to rte_power_monitor at
>>>>>> invocation time.
>>>>>>
>>>>>> The latter doesn't work well because when we call rte_power_monitor from
>>>>>> inside the rte_power library, we lack the context necessary to get said
>>>>>> opaque pointer.
>>>>>>
>>>>>> The former doesn't work either, because the only place where we can get
>>>>>> this argument is inside get_monitor_addr, but the opaque pointer must
>>>>>> persist after we exit that function in order to avoid use-after-free -
>>>>>> which means that it either has to be statically allocated (which means
>>>>>> it's not thread-safe for a non-trivial case), or dynamically allocated
>>>>>> (which a big no-no on a hotpath).
>>>>>
>>>>> If I get you right, expected_value (and probably mask) can be variable ones.
>>>>> So for callback approach to work we need to pass all this as parameters
>>>>> to PMD comparison callback:
>>>>> int pmc_callback(uint64_t real_val, uint64_t expected_val, uint64_t mask)
>>>>> Correct?
>>>>
>>>> If we have both expected value, mask, and current value, then what's the
>>>> point of the callback? The point of the callback would be to pass just
>>>> the current value, and let the callback decide what's the expected value
>>>> and how to compare it.
>>>
>>> For me the main point of callback is to hide PMD specific comparison semantics.
>>> Basically they provide us with some values in struct rte_power_monitor_cond,
>>> and then it is up to them how to interpret them in their comparison function.
>>> All we'll do for them: will read the value at address provided.
>>> I understand that it looks like an overkill, as majority of these comparison functions
>>> will be like:
>>> int cmp_callback(uint64_t real_val, uint64_t expected_val, uint64_t mask)
>>> {
>>> return ((real_val & mask) == expected_val);
>>> }
>>> Though qsort() and bsearch() work in a similar manner, and everyone seems ok with it.
>>>
>>>>
>>>> So, we can either let callback handle expected values itself by having
>>>> an opaque callback-specific argument (which means it has to persist
>>>> between .get_monitor_addr() and rte_power_monitor() calls),
>>>
>>> But that's what we doing already - PMD fills rte_power_monitor_cond values
>>> for us, we store them somewhere and then use them to decide should we go to sleep or not.
>>> All callback does - moves actual values interpretation back to PMD:
>>> Right now:
>>> PMD: provide PMC values
>>> POWER: store PMC values somewhere
>>> read the value at address provided in PMC
>>> interpret PMC values and newly read value and make the decision
>>>
>>> With callback:
>>> PMD: provide PMC values
>>> POWER: store PMC values somewhere
>>> read the value at address provided in PMC
>>> PMD: interpret PMC values and newly read value and make the decision
>>>
>>> Or did you mean something different here?
>>>
>>>> or we do the
>>>> comparisons inside rte_power_monitor(), and store the expected/mask
>>>> values in the monitor condition, and *don't* have any callbacks at all.
>>>> Are you suggesting an alternative to the above two options?
>>>
>>> As I said in my first mail - we can just replace 'inverse' with 'op'.
>>> That at least will make this API extendable, if someone will need
>>> something different in future.
>>>
>>> Another option is
>>
>> Right, so the idea is store the PMD-specific data in the monitor
>> condition, and leave it to the callback to interpret it.
>>
>> The obvious question then is, how many values is enough? Two? Three?
>> Four? This option doesn't really solve the basic issue, it just kicks
>> the can down the road. I guess three values should be enough for
>> everyone (tm) ? :D
>>
>> I don't like the 'op' thing because if the goal is to be flexible, it's
>> unnecessarily limiting *and* makes the API even more complex to use. I
>> would rather have a number of PMD-specific values and leave it up to the
>> callback to interpret them, because at least that way we're not limited
>> to predefined operations on the monitor condition data.
>
> Just to make sure we are talking about the same, does what you propose
> looks like that:
>
> struct rte_power_monitor_cond {
> volatile void *addr; /**< Address to monitor for changes */
> uint8_t size; /**< Data size (in bytes) that will be used to compare
> * expected value (`val`) with data read from the
> * monitored memory location (`addr`). Can be 1, 2,
> * 4, or 8. Supplying any other value will result in
> * an error.
> */
> int (*cmp)(uint64_t real_value, const uint64_t opaque[4]);
> uint64_t opaque[4]; /*PMD specific data, used by comparison call-back below */
> };
>
> And then in rte_power_monitor():
> ...
> uint64_t cur_value = __get_umwait_val(pmc->addr, pmc->size);
> if (pmc->cmp(cur_value, pmc->opaque) != 0) {
> /* goto sleep */
> }
>
> ?
>
Something like that, yes.
--
Thanks,
Anatoly
next prev parent reply other threads:[~2021-06-24 15:04 UTC|newest]
Thread overview: 165+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-01 12:00 [dpdk-dev] [PATCH v1 0/7] Enhancements for PMD power management Anatoly Burakov
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 1/7] power_intrinsics: allow monitor checks inversion Anatoly Burakov
2021-06-21 12:56 ` Ananyev, Konstantin
2021-06-23 9:43 ` Burakov, Anatoly
2021-06-23 9:55 ` Ananyev, Konstantin
2021-06-23 10:00 ` Burakov, Anatoly
2021-06-23 11:00 ` Ananyev, Konstantin
2021-06-23 12:12 ` Burakov, Anatoly
2021-06-23 13:27 ` Ananyev, Konstantin
2021-06-23 14:13 ` Burakov, Anatoly
2021-06-24 9:47 ` Ananyev, Konstantin
2021-06-24 14:34 ` Burakov, Anatoly
2021-06-24 14:57 ` Ananyev, Konstantin
2021-06-24 15:04 ` Burakov, Anatoly [this message]
2021-06-24 15:25 ` Ananyev, Konstantin
2021-06-24 15:54 ` Burakov, Anatoly
2021-07-09 15:03 ` David Marchand
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-06-02 12:59 ` Loftus, Ciara
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-06-22 9:13 ` Ananyev, Konstantin
2021-06-23 9:46 ` Burakov, Anatoly
2021-06-23 9:52 ` Ananyev, Konstantin
2021-06-25 11:52 ` Burakov, Anatoly
2021-06-25 14:42 ` Ananyev, Konstantin
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-06-22 9:41 ` Ananyev, Konstantin
2021-06-23 9:36 ` Burakov, Anatoly
2021-06-23 9:49 ` Ananyev, Konstantin
2021-06-23 9:56 ` Burakov, Anatoly
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 6/7] power: support monitoring " Anatoly Burakov
2021-06-01 12:00 ` [dpdk-dev] [PATCH v1 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 0/7] Enhancements for PMD power management Anatoly Burakov
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-06-28 12:19 ` Ananyev, Konstantin
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-06-28 12:37 ` Ananyev, Konstantin
2021-06-28 12:43 ` Burakov, Anatoly
2021-06-28 12:58 ` Ananyev, Konstantin
2021-06-28 13:29 ` Burakov, Anatoly
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-06-28 7:10 ` David Marchand
2021-06-28 9:25 ` Burakov, Anatoly
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 6/7] power: support monitoring " Anatoly Burakov
2021-06-25 14:00 ` [dpdk-dev] [PATCH v2 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 0/7] Enhancements for PMD power management Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 6/7] power: support monitoring " Anatoly Burakov
2021-06-28 13:29 ` Ananyev, Konstantin
2021-06-28 14:09 ` Burakov, Anatoly
2021-06-29 0:07 ` Ananyev, Konstantin
2021-06-29 11:05 ` Burakov, Anatoly
2021-06-29 11:39 ` Burakov, Anatoly
2021-06-29 12:14 ` Ananyev, Konstantin
2021-06-29 13:23 ` Burakov, Anatoly
2021-06-28 12:41 ` [dpdk-dev] [PATCH v3 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 0/7] Enhancements for PMD power management Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 6/7] power: support monitoring " Anatoly Burakov
2021-06-28 15:54 ` [dpdk-dev] [PATCH v4 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 0/7] Enhancements for PMD power management Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-06-30 9:52 ` David Hunt
2021-07-01 9:01 ` David Hunt
2021-07-05 10:24 ` Burakov, Anatoly
2021-06-30 11:04 ` Ananyev, Konstantin
2021-07-05 10:23 ` Burakov, Anatoly
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 6/7] power: support monitoring " Anatoly Burakov
2021-06-30 10:29 ` Ananyev, Konstantin
2021-07-05 10:08 ` Burakov, Anatoly
2021-06-29 15:48 ` [dpdk-dev] [PATCH v5 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-07-05 15:21 ` [dpdk-dev] [PATCH v6 0/7] Enhancements for PMD power management Anatoly Burakov
2021-07-05 15:21 ` [dpdk-dev] [PATCH v6 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-07-05 15:21 ` [dpdk-dev] [PATCH v6 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-07-05 15:21 ` [dpdk-dev] [PATCH v6 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-08-04 9:52 ` Kinsella, Ray
2021-07-05 15:21 ` [dpdk-dev] [PATCH v6 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-07-07 10:14 ` Ananyev, Konstantin
2021-07-05 15:22 ` [dpdk-dev] [PATCH v6 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-07-06 18:50 ` Ananyev, Konstantin
2021-07-07 10:06 ` Burakov, Anatoly
2021-07-07 10:11 ` Ananyev, Konstantin
2021-07-07 11:54 ` Burakov, Anatoly
2021-07-07 12:51 ` Ananyev, Konstantin
2021-07-07 14:35 ` Burakov, Anatoly
2021-07-07 17:09 ` Ananyev, Konstantin
2021-07-07 10:04 ` David Hunt
2021-07-07 10:28 ` Burakov, Anatoly
2021-07-05 15:22 ` [dpdk-dev] [PATCH v6 6/7] power: support monitoring " Anatoly Burakov
2021-07-07 10:16 ` Ananyev, Konstantin
2021-07-05 15:22 ` [dpdk-dev] [PATCH v6 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 0/7] Enhancements for PMD power management Anatoly Burakov
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-07-07 11:56 ` David Hunt
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-07-07 12:01 ` David Hunt
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-07-07 12:02 ` David Hunt
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-07-07 11:54 ` David Hunt
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 6/7] power: support monitoring " Anatoly Burakov
2021-07-07 12:03 ` David Hunt
2021-07-07 10:48 ` [dpdk-dev] [PATCH v7 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-07-07 12:03 ` David Hunt
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 0/7] Enhancements for PMD power management Anatoly Burakov
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 1/7] power_intrinsics: use callbacks for comparison Anatoly Burakov
2021-07-08 16:56 ` McDaniel, Timothy
2021-07-09 13:46 ` Thomas Monjalon
2021-07-09 14:41 ` Burakov, Anatoly
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 2/7] net/af_xdp: add power monitor support Anatoly Burakov
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 3/7] eal: add power monitor for multiple events Anatoly Burakov
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 4/7] power: remove thread safety from PMD power API's Anatoly Burakov
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 5/7] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-07-09 14:24 ` David Marchand
2021-07-09 14:42 ` Burakov, Anatoly
2021-07-09 14:46 ` David Marchand
2021-07-09 14:53 ` Burakov, Anatoly
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 6/7] power: support monitoring " Anatoly Burakov
2021-07-08 14:13 ` [dpdk-dev] [PATCH v8 7/7] l3fwd-power: support multiqueue in PMD pmgmt modes Anatoly Burakov
2021-07-09 14:50 ` David Marchand
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 0/8] Enhancements for PMD power management Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 1/8] eal: use callbacks for power monitoring comparison Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 2/8] net/af_xdp: add power monitor support Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 3/8] doc: add PMD power management NIC feature Anatoly Burakov
2021-07-09 15:57 ` Burakov, Anatoly
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 4/8] eal: add power monitor for multiple events Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 5/8] power: remove thread safety from PMD power API's Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 6/8] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 7/8] power: support monitoring " Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 15:53 ` [dpdk-dev] [PATCH v9 8/8] examples/l3fwd-power: support multiq in PMD modes Anatoly Burakov
2021-07-09 16:00 ` Anatoly Burakov
2021-07-09 16:00 ` [dpdk-dev] [PATCH v9 0/8] Enhancements for PMD power management Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 " Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 1/8] eal: use callbacks for power monitoring comparison Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 2/8] net/af_xdp: add power monitor support Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 3/8] doc: add PMD power management NIC feature Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 4/8] eal: add power monitor for multiple events Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 5/8] power: remove thread safety from PMD power API's Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 6/8] power: support callbacks for multiple Rx queues Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 7/8] power: support monitoring " Anatoly Burakov
2021-07-09 16:08 ` [dpdk-dev] [PATCH v10 8/8] examples/l3fwd-power: support multiq in PMD modes Anatoly Burakov
2021-07-09 19:24 ` [dpdk-dev] [PATCH v10 0/8] Enhancements for PMD power management David Marchand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=882380b2-20b9-f081-9a5a-199c38094b1b@intel.com \
--to=anatoly.burakov@intel.com \
--cc=bruce.richardson@intel.com \
--cc=ciara.loftus@intel.com \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).