[dpdk-dev] [PATCH v2 1/7] power_intrinsics: use callbacks for comparison
Ananyev, Konstantin
konstantin.ananyev at intel.com
Mon Jun 28 14:19:41 CEST 2021
> 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 replaces the comparison with a user callback mechanism, so
> that any PMD (or other code) using `rte_power_monitor()` can define
> their own comparison semantics and decision making on how to detect the
> need to abort the entering of power optimized state.
>
> Existing implementations are adjusted to follow the new semantics.
>
> Suggested-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> ---
>
> Notes:
> v2:
> - Use callback mechanism for more flexibility
> - Address feedback from Konstantin
>
> doc/guides/rel_notes/release_21_08.rst | 1 +
> drivers/event/dlb2/dlb2.c | 16 ++++++++--
> drivers/net/i40e/i40e_rxtx.c | 19 ++++++++----
> drivers/net/iavf/iavf_rxtx.c | 19 ++++++++----
> drivers/net/ice/ice_rxtx.c | 19 ++++++++----
> drivers/net/ixgbe/ixgbe_rxtx.c | 19 ++++++++----
> drivers/net/mlx5/mlx5_rx.c | 16 ++++++++--
> .../include/generic/rte_power_intrinsics.h | 29 ++++++++++++++-----
> lib/eal/x86/rte_power_intrinsics.c | 9 ++----
> 9 files changed, 106 insertions(+), 41 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_power_intrinsics.h b/lib/eal/include/generic/rte_power_intrinsics.h
> index dddca3d41c..046667ade6 100644
> --- a/lib/eal/include/generic/rte_power_intrinsics.h
> +++ b/lib/eal/include/generic/rte_power_intrinsics.h
> @@ -18,19 +18,34 @@
> * which are architecture-dependent.
> */
>
> +/**
> + * Callback definition for monitoring conditions. Callbacks with this signature
> + * will be used by `rte_power_monitor()` to check if the entering of power
> + * optimized state should be aborted.
> + *
> + * @param val
> + * The value read from memory.
> + * @param opaque
> + * Callback-specific data.
> + *
> + * @return
> + * 0 if entering of power optimized state should proceed
> + * -1 if entering of power optimized state should be aborted
> + */
> +typedef int (*rte_power_monitor_clb_t)(const uint64_t val,
> + const uint64_t opaque[4]);
> struct rte_power_monitor_cond {
> volatile void *addr; /**< Address to monitor for changes */
> - uint64_t val; /**< If the `mask` is non-zero, location pointed
> - * to by `addr` will be read and compared
> - * against this value.
> - */
> - uint64_t mask; /**< 64-bit mask to extract value read from `addr` */
> - uint8_t size; /**< Data size (in bytes) that will be used to compare
> - * expected value (`val`) with data read from the
> + uint8_t size; /**< Data size (in bytes) that will be read from the
> * monitored memory location (`addr`). Can be 1, 2,
> * 4, or 8. Supplying any other value will result in
> * an error.
> */
> + rte_power_monitor_clb_t fn; /**< Callback to be used to check if
> + * entering power optimized state should
> + * be aborted.
> + */
> + uint64_t opaque[4]; /**< Callback-specific data */
As a nit - would be good to add some new macro for '4'.
Apart from that - LGTM.
Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
> };
>
> /**
> diff --git a/lib/eal/x86/rte_power_intrinsics.c b/lib/eal/x86/rte_power_intrinsics.c
> index 39ea9fdecd..3c5c9ce7ad 100644
> --- a/lib/eal/x86/rte_power_intrinsics.c
> +++ b/lib/eal/x86/rte_power_intrinsics.c
> @@ -110,14 +110,11 @@ rte_power_monitor(const struct rte_power_monitor_cond *pmc,
> /* now that we've put this address into monitor, we can unlock */
> rte_spinlock_unlock(&s->lock);
>
> - /* if we have a comparison mask, we might not need to sleep at all */
> - if (pmc->mask) {
> + /* if we have a callback, we might not need to sleep at all */
> + if (pmc->fn) {
> const uint64_t cur_value = __get_umwait_val(
> pmc->addr, pmc->size);
> - const uint64_t masked = cur_value & pmc->mask;
> -
> - /* if the masked value is already matching, abort */
> - if (masked == pmc->val)
> + if (pmc->fn(cur_value, pmc->opaque) != 0)
> goto end;
> }
>
> --
> 2.25.1
More information about the dev
mailing list