Hi Stephen,
> -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Thursday, April 2, 2020 11:48 PM > To: Joyce Kong <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; Honnappa Nagarahalli > <[email protected]>; Gavin Hu <[email protected]>; nd > <[email protected]>; [email protected] > Subject: Re: [dpdk-dev] [PATCH v2 1/2] virtio: one way barrier for split vring > used idx > > On Thu, 2 Apr 2020 10:57:52 +0800 > Joyce Kong <[email protected]> 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 Will fix in v3. > > > +/* 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). Will fix in v3. > > > +#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. Will fix in v3. > > > +} > > This kind of arch specific code is hard to maintain. > Does it really make that much difference. Yes, a stronger than required barrier is a performance killer, especially in the fast path. The test was conducted on the ThunderX2+Intel XL710 testbed, the PVP test case. /Gavin
