[ovs-dev] [PATCH v9 2/3] dpif-netdev: Detailed performance stats for PMDs

[Ilya Maximets]

On 15.03.2018 20:04, Jan Scheurich wrote:
>>> +static inline bool
>>> +pmd_perf_metrics_enabled(const struct dp_netdev_pmd_thread *pmd)
>>> +{
>>> +    /* If stores and reads of 64-bit integers are not atomic, the
>>> +     * full PMD performance metrics are not available as locked
>>> +     * access to 64 bit integers would be prohibitively expensive. */
>>> +    if (sizeof(uint64_t) > sizeof(void *)) {
>>> +        return false;
>>> +    }
>>
>>
>> Hmm. And what about using pre-defined macroses here instead of manual
>> pointer checking?
>> Did you forget about this comment?
> 
> Yes, I missed that one. Too longs since and too much other things in between :-(
> 
> Is that what you would like to see instead?

Yes, kind of.

> 
> static inline bool
> pmd_perf_metrics_enabled(const struct dp_netdev_pmd_thread *pmd)
> {
>     /* If stores and reads of 64-bit integers are not atomic, the
>      * full PMD performance metrics are not available as locked
>      * access to 64 bit integers would be prohibitively expensive. */
> #if ATOMIC_LLONG_LOCK_FREE
>     bool pmd_perf_enabled;
>     atomic_read_relaxed(&pmd->dp->pmd_perf_metrics, &pmd_perf_enabled);
>     return pmd_perf_enabled;
> #else
>     return false;
> #endif
> }
> 
> Are you planning to do another full review of the series? If so, I'd wait with v10 for your comments on v9.
> Otherwise I can send v10 with the fix straight away.
> 

Sorry for late response. I'm planning to spend some time reviewing v10 during this week.

Best regards, Ilya Maximets.