DPDK patches and discussions
 help / color / mirror / Atom feed
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: Wed, 23 Jun 2021 11:00:39 +0100	[thread overview]
Message-ID: <249f6957-0f10-926f-9f0e-5cb10fc4d2c3@intel.com> (raw)
In-Reply-To: <DM6PR11MB4491A8C6C27B2DFBE35F99D49A089@DM6PR11MB4491.namprd11.prod.outlook.com>

On 23-Jun-21 10:55 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Sent: Wednesday, June 23, 2021 10:43 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
>> Cc: Loftus, Ciara <ciara.loftus@intel.com>; Hunt, David <david.hunt@intel.com>
>> Subject: Re: [PATCH v1 1/7] power_intrinsics: allow monitor checks inversion
>>
>> On 21-Jun-21 1:56 PM, Ananyev, Konstantin wrote:
>>>
>>> Hi Anatoly,
>>>
>>>> Previously, the semantics of power monitor were such that we were
>>>> checking current value against the expected value, and if they matched,
>>>> then the sleep was aborted. This is somewhat inflexible, because it only
>>>> allowed us to check for a specific value.
>>>>
>>>> This commit adds an option to reverse the check, so that we can have
>>>> monitor sleep aborted if the expected value *doesn't* match what's in
>>>> memory. This allows us to both implement all currently implemented
>>>> driver code, as well as support more use cases which don't easily map to
>>>> previous semantics (such as waiting on writes to AF_XDP counter value).
>>>>
>>>> Since the old behavior is the default, no need to adjust existing
>>>> implementations.
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>>    lib/eal/include/generic/rte_power_intrinsics.h | 4 ++++
>>>>    lib/eal/x86/rte_power_intrinsics.c             | 5 ++++-
>>>>    2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/eal/include/generic/rte_power_intrinsics.h b/lib/eal/include/generic/rte_power_intrinsics.h
>>>> index dddca3d41c..1006c2edfc 100644
>>>> --- a/lib/eal/include/generic/rte_power_intrinsics.h
>>>> +++ b/lib/eal/include/generic/rte_power_intrinsics.h
>>>> @@ -31,6 +31,10 @@ struct rte_power_monitor_cond {
>>>>                           *   4, or 8. Supplying any other value will result in
>>>>                           *   an error.
>>>>                           */
>>>> +     uint8_t invert;  /**< Invert check for expected value (e.g. instead of
>>>> +                       *   checking if `val` matches something, check if
>>>> +                       *   `val` *doesn't* match a particular value)
>>>> +                       */
>>>>    };
>>>>
>>>>    /**
>>>> diff --git a/lib/eal/x86/rte_power_intrinsics.c b/lib/eal/x86/rte_power_intrinsics.c
>>>> index 39ea9fdecd..5d944e9aa4 100644
>>>> --- a/lib/eal/x86/rte_power_intrinsics.c
>>>> +++ b/lib/eal/x86/rte_power_intrinsics.c
>>>> @@ -117,7 +117,10 @@ rte_power_monitor(const struct rte_power_monitor_cond *pmc,
>>>>                 const uint64_t masked = cur_value & pmc->mask;
>>>>
>>>>                 /* if the masked value is already matching, abort */
>>>> -             if (masked == pmc->val)
>>>> +             if (!pmc->invert && masked == pmc->val)
>>>> +                     goto end;
>>>> +             /* same, but for inverse check */
>>>> +             if (pmc->invert && masked != pmc->val)
>>>>                         goto end;
>>>>         }
>>>>
>>>
>>> Hmm..., such approach looks too 'patchy'...
>>> Can we at least replace 'inver' with something like:
>>> enum rte_power_monitor_cond_op {
>>>           _EQ, NEQ,...
>>> };
>>> Then at least new comparions ops can be added in future.
>>> Even better I think would be to just leave to PMD to provide a comparison callback.
>>> Will make things really simple and generic:
>>> struct rte_power_monitor_cond {
>>>        volatile void *addr;
>>>        int (*cmp)(uint64_t val);
>>>        uint8_t size;
>>> };
>>> And then in rte_power_monitor(...):
>>> ....
>>> const uint64_t cur_value = __get_umwait_val(pmc->addr, pmc->size);
>>> if (pmc->cmp(cur_value) != 0)
>>>           goto end;
>>> ....
>>>
>>
>> I like the idea of a callback, but these are supposed to be
>> intrinsic-like functions, so putting too much into them is contrary to
>> their goal, and it's going to make the API hard to use in simpler cases
>> (e.g. when we're explicitly calling rte_power_monitor as opposed to
>> letting the RX callback do it for us). For example, event/dlb code calls
>> rte_power_monitor explicitly.
> 
> Good point, I didn't know that.
> Would be interesting to see how do they use it.

To be fair, it should be possible to rewrite their code using a 
callback. Perhaps adding a (void *) parameter for any custom data 
related to the callback (because C doesn't have closures...), but 
otherwise it should be doable, so the question isn't that it's 
impossible to rewrite event/dlb code to use callbacks, it's more of an 
issue with complicating usage of already-not-quite-straightforward API 
even more.

> 
>>
>> It's going to be especially "fun" to do these indirect function calls
>> from inside transactional region on call to multi-monitor.
> 
> But the callback is not supposed to do any memory reads/writes.
> Just mask/compare of the provided value with some constant.

Yeah, but with callbacks we can't really control that, can we? I mean i 
guess a *sane* implementation wouldn't do that, but still, it's 
theoretically possible to perform more complex checks and even touch 
some unrelated data in the process.

> 
>> I'm not
>> opposed to having a callback here, but maybe others have more thoughts
>> on this?
>>
>> --
>> Thanks,
>> Anatoly


-- 
Thanks,
Anatoly

  reply	other threads:[~2021-06-23 10:00 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 [this message]
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
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=249f6957-0f10-926f-9f0e-5cb10fc4d2c3@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).