Re: [Qemu-devel] [PATCH 09/12] ring: introduce lockless ring buffer

[Xiao Guangrong]

On 06/29/2018 09:08 PM, Michael S. Tsirkin wrote:
> On Fri, Jun 29, 2018 at 03:30:44PM +0800, Xiao Guangrong wrote:
> > Hi Michael,
> >
> > On 06/20/2018 08:38 PM, Michael S. Tsirkin wrote:
> > > On Mon, Jun 04, 2018 at 05:55:17PM +0800, address@hidden wrote:
> > > > From: Xiao Guangrong <address@hidden>
> >
> > > > (1) 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/kfifo.h
> > > > (2) http://dpdk.org/doc/api/rte__ring_8h.html
> > > >
> > > > Signed-off-by: Xiao Guangrong <address@hidden>
> > >
> > > So instead of all this super-optimized trickiness, how about
> > > a simple port of ptr_ring from linux?
> > >
> > > That one isn't lockless but it's known to outperform
> > > most others for a single producer/single consumer case.
> > > And with a ton of networking going on,
> > > who said it's such a hot spot? OTOH this implementation
> > > has more barriers which slows down each individual thread.
> > > It's also a source of bugs.
> >
> > Thank you for pointing it out.
> >
> > I just quickly went through the code of ptr_ring that is very nice and
> > really impressive. I will consider to port it to QEMU.
>
> The port is pretty trivial. See below. It's a SPSC structure though.  So
> you need to use it with lock.  Given the critical section is small, I

Why put these locks into this common struct? For our case, each thread
has its own ring which is SCSP, no lock is needed at all. Atomic operations
still slow things down, see [PATCH 07/12] migration: hold the lock only if
it is really needed. I'd move the inner locks to the user instead.