> -----Original Message-----
> From: [email protected] [mailto:ovs-dev-
> [email protected]] On Behalf Of Kevin Traynor
> Sent: Friday, May 5, 2017 5:34 PM
> To: [email protected]
> Subject: [ovs-dev] [RFC PATCH 1/6] dpif-netdev: Add rxq processing cycle
> counters.
>
> Add two counters to dp_netdev_rxq which will be used for storing the
> processing cycles of the rxq. Processing cycles will stored in reference
> to a defined interval. One counter is used to count cycles during the
> current in progress interval, while the other is used to store the cycles
> of the last complete interval.
>
> Signed-off-by: Kevin Traynor <[email protected]>
> ---
> lib/dpif-netdev.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 7352d6f..d2a02af
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -343,4 +343,6 @@ struct dp_netdev_rxq {
> particular core. */
> struct dp_netdev_pmd_thread *pmd; /* pmd thread that will poll this
> queue. */
> + atomic_ullong cyc_curr; /* Current pmd interval proc
> cycles. */
> + atomic_ullong cyc_last; /* Last pmd interval proc cycles.
> */
> };
>
> @@ -665,4 +667,14 @@ static void pmd_load_cached_ports(struct
> dp_netdev_pmd_thread *pmd) static inline void
> dp_netdev_pmd_try_optimize(struct dp_netdev_pmd_thread *pmd);
> +static void
> +dp_netdev_rxq_set_cyc_curr(struct dp_netdev_rxq *rx,
> + unsigned long long cycles); static uint64_t
> +dp_netdev_rxq_get_cyc_curr(const struct dp_netdev_rxq *rx); static void
> +dp_netdev_rxq_set_cyc_last(struct dp_netdev_rxq *rx,
> + unsigned long long cycles); static
> +uint64_t dp_netdev_rxq_get_cyc_last(const struct dp_netdev_rxq *rx);
>
> static void
> @@ -3046,4 +3058,35 @@ cycles_count_intermediate(struct
> dp_netdev_pmd_thread *pmd, }
>
> +static void
> +dp_netdev_rxq_set_cyc_curr(struct dp_netdev_rxq *rx,
> + unsigned long long cycles) {
> + atomic_store_relaxed(&rx->cyc_curr, cycles); }
> +
> +static uint64_t
> +dp_netdev_rxq_get_cyc_curr(const struct dp_netdev_rxq *rx) {
> + unsigned long long tmp;
> + atomic_read_relaxed(&rx->cyc_curr, &tmp);
> + return tmp;
> +}
> +
> +static void
> +dp_netdev_rxq_set_cyc_last(struct dp_netdev_rxq *rx,
> + unsigned long long cycles) {
> + atomic_store_relaxed(&rx->cyc_last, cycles); }
> +
> +static uint64_t
> +dp_netdev_rxq_get_cyc_last(const struct dp_netdev_rxq *rx) {
> + unsigned long long tmp;
> + atomic_read_relaxed(&rx->cyc_last, &tmp);
> + return tmp;
> +}
> +
Some code duplication above for the get/set methods above.
I was thinking could you have 1 function for dp_netdev_rxq_set_cyc and 1 for
dp_netdev_rxq_get_cyc and use an enum to signal if its 'cyc_last' or
'cyc_curr'. But a concern with this approach is that the logical decision on
the enum type will impact negatively on performance.
I know this is an RFC so it probably doesn't matter but as a heads up you'll
get compilation warnings of the functions not being used as is after applying
this patch. Maybe something to consider when rolling out the next patchset.
Also clang complains of the following:
lib/dpif-netdev.c:3071:5: error: address argument to atomic operation must be a
pointer to non-const _Atomic type ('const atomic_ullong *' (aka 'const
_Atomic(unsigned long long) *') invalid)
atomic_read_relaxed(&rx->cyc_curr, &tmp);
^ ~~~~~~~~~~~~~
./lib/ovs-atomic.h:395:5: note: expanded from macro 'atomic_read_relaxed'
atomic_read_explicit(VAR, DST, memory_order_relaxed)
^ ~~~
./lib/ovs-atomic-clang.h:53:15: note: expanded from macro
'atomic_read_explicit'
(*(DST) = __c11_atomic_load(SRC, ORDER), \
^ ~~~
lib/dpif-netdev.c:3086:5: error: address argument to atomic operation must be a
pointer to non-const _Atomic type ('const atomic_ullong *' (aka 'const
_Atomic(unsigned long long) *') invalid)
atomic_read_relaxed(&rx->cyc_last, &tmp);
^ ~~~~~~~~~~~~~
./lib/ovs-atomic.h:395:5: note: expanded from macro 'atomic_read_relaxed'
atomic_read_explicit(VAR, DST, memory_order_relaxed)
^ ~~~
./lib/ovs-atomic-clang.h:53:15: note: expanded from macro
'atomic_read_explicit'
(*(DST) = __c11_atomic_load(SRC, ORDER), \
Sparse throws errors for the same issue
lib/dpif-netdev.c:3075:5: warning: incorrect type in argument 1 (different
modifiers)
lib/dpif-netdev.c:3075:5: expected void *<noident>
lib/dpif-netdev.c:3075:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3075:5: warning: incorrect type in argument 1 (different
modifiers)
lib/dpif-netdev.c:3075:5: expected void *<noident>
lib/dpif-netdev.c:3075:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3090:5: warning: incorrect type in argument 1 (different
modifiers)
lib/dpif-netdev.c:3090:5: expected void *<noident>
lib/dpif-netdev.c:3090:5: got unsigned long long const *<noident>
lib/dpif-netdev.c:3090:5: warning: incorrect type in argument 1 (different
modifiers)
lib/dpif-netdev.c:3090:5: expected void *<noident>
lib/dpif-netdev.c:3090:5: got unsigned long long const *<noident>
> +
> static int
> dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
