DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Burakov, Anatoly" <anatoly.burakov@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 14:57:17 +0000	[thread overview]
Message-ID: <DM6PR11MB44918C1C1C82EDD5DD3715AB9A079@DM6PR11MB4491.namprd11.prod.outlook.com> (raw)
In-Reply-To: <e1c69e40-120b-1bae-5698-0f15e0af711e@intel.com>



> >>>> 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 */
}

?
 

  reply	other threads:[~2021-06-24 14:57 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 [this message]
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=DM6PR11MB44918C1C1C82EDD5DD3715AB9A079@DM6PR11MB4491.namprd11.prod.outlook.com \
    --to=konstantin.ananyev@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=ciara.loftus@intel.com \
    --cc=david.hunt@intel.com \
    --cc=dev@dpdk.org \
    /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).