Re: [dpdk-dev] [PATCH v2 1/2] virtio: one way barrier for split vring used idx

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu,  2 Apr 2020 10:57:52 +0800
Joyce Kong <joyce.kong@arm.com> wrote:

> -					(vq)->vq_used_cons_idx))
> +static inline uint16_t
> +virtqueue_nused(struct virtqueue *vq)
vq is unmodified and should be const

> +{
> +	uint16_t idx;
> +	if (vq->hw->weak_barriers) {
Put blank line between declaration and if statement

> +/* x86 prefers to using rte_smp_rmb over __atomic_load_n as it reports
> + * a slightly better perf, which comes from the saved branch by the compiler.
> + * The if and else branches are identical with the smp and cio barriers both
> + * defined as compiler barriers on x86.
> + */

Do not put comments on left margin (except in function prolog).

> +#ifdef RTE_ARCH_X86_64
> +		idx = vq->vq_split.ring.used->idx;
> +		rte_smp_rmb();
> +#else
> +		idx = __atomic_load_n(&(vq)->vq_split.ring.used->idx,
> +				__ATOMIC_ACQUIRE);
> +#endif
> +	} else {
> +		idx = vq->vq_split.ring.used->idx;
> +		rte_cio_rmb();
> +	}
> +	return (idx - vq->vq_used_cons_idx);

Parenthesis around arguments to return are unnecessary.
BSD code likes it, Linux style does not.

> +}

This kind of arch specific code is hard to maintain.
Does it really make that much difference.