Re: [dpdk-dev] [PATCH v4] vhost_user: protect active rings from async ring changes

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 20 Dec 2017 15:06:30 -0500 (EST)
Victor Kaplansky <vkaplans@redhat.com> wrote:

> > Wrapping locking inline's adds nothing and makes life harder
> > for static analysis tools.  
> 
> Yep. In this case it inhibits the details of how the locking is
> implemented (e.g. the name of the lock). It also facilitates
> replacement of locking mechanism, by another implementation.
> See below.

YAGNI You aren't gonna need it.

Don't build infrastructure for things that you forsee.




> > 
> > The bigger problem is that doing locking on all enqueue/dequeue
> > can have a visible performance impact. Did you measure that?
> > 
> > Could you invent an RCUish mechanism using compiler barriers?
> >   
> 
> I've played a bit with measuring performance impact. Successful
> lock adds on the average about 30 cycles on my Haswell cpu.
> (and it successes 99.999...% of time).
> 
> I can investigate it more, but my initial feeling is that adding a
> memory barrier (the real one, not the compiler barrier) would add
> about the same overhead.
> 
> By the way, the way update_queuing_status() in 
> drivers/net/vhost/rte_eth_vhost.c tries to avoid contention with
> the active queue by playing with "allow_queuing" and "while_queuing"
> seems to be broken, since memory barriers are missing.

CPU cycles alone don't matter on modern x86.
What matters is cache and instructions per cycle.
In this case locking requires locked instruction which causes the cpu
prefetching and instruction pipeline to stall.