RE: [PATCH] ring: add cache guard after ring elements table

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> > > > > Added cache guard after the table holding the ring elements, to
> > avoid
> > > > > false sharing conflicts caused by next-line hardware prefetchers
> > when
> > > > > accessing elements at the end of the ring table.
> > > >
> > > > I don't see any harm with it, and in theory it might help in some
> > > > cases...
> > > > Though I wonder how real is that problem?
> > > > Did you ever observe such contention to happen?
> > >
> > > I never observed a problem with this.
> > > The risk of contention depends on what is allocated in the memory
> > after the ring.
> > > Which is application specific.
> > >
> > > It seems like a purely theoretical issue, but should be fixed anyway,
> > to eliminate
> > > that risk.
> >
> > Ok, as I said I see no harm with it.
> > Should we document this change somewhere? RN or PG?
> 
> We don't want the release notes overflowing with minor details.
> IMO, this change is below the threshold for what people might care about.
> People interested in the detailed changes between releases should read the git
> log.

I still think we do need document somewhere why we doing it.
If you think RN or PG is not the right place, let's just put it as a comment for that particular function.

> Also, I don't think it's worth backporting, because I consider it unlikely to have
> any real effect.
> In the context of backporting, it could be considered a performance
> improvement rather than a bug fix.

I don't see much point to backport it.

> 
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> >
> > > >
> > > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > > > ---
> > > > >  lib/ring/rte_ring.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> > > > > index f10050a1c4..9ccc62cd42 100644
> > > > > --- a/lib/ring/rte_ring.c
> > > > > +++ b/lib/ring/rte_ring.c
> > > > > @@ -73,8 +73,11 @@ rte_ring_get_memsize_elem(unsigned int esize,
> > > > unsigned
> > > > > int count)
> > > > >  		return -EINVAL;
> > > > >  	}
> > > > >
> > > > > +	static_assert(sizeof(struct rte_ring) ==
> > > > > RTE_CACHE_LINE_ROUNDUP(sizeof(struct rte_ring)),
> > > > > +			"Size of struct rte_ring not cache line
> > aligned");
> > > > >  	sz = sizeof(struct rte_ring) + (ssize_t)count * esize;
> > > > >  	sz = RTE_ALIGN(sz, RTE_CACHE_LINE_SIZE);
> > > > > +	sz += RTE_CACHE_GUARD_LINES * RTE_CACHE_LINE_SIZE;
> > > > >  	return sz;
> > > > >  }
> > > > >
> > > > > --
> > > > > 2.43.0