[PATCH v2 1/2] x86: Improve large memset perf with non-temporal stores [RHEL-29312]

[Sunil Pandey]

On Mon, Jul 15, 2024 at 1:33 AM Noah Goldstein <goldstein.w.n@gmail.com>
wrote:

> On Mon, Jul 15, 2024 at 3:48 PM Noah Goldstein <goldstein.w.n@gmail.com>
> wrote:
> >
> > On Mon, Jul 15, 2024 at 3:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > >
> > > On Mon, Jul 15, 2024, 3:36 PM Noah Goldstein <goldstein.w.n@gmail.com>
> wrote:
> > >>
> > >> On Fri, Jul 12, 2024 at 3:04 PM Noah Goldstein <
> goldstein.w.n@gmail.com> wrote:
> > >> >
> > >> > On Fri, Jul 12, 2024 at 4:56 AM DJ Delorie <dj@redhat.com> wrote:
> > >> > >
> > >> > >
> > >> > > Noah Goldstein <goldstein.w.n@gmail.com> writes:
> > >> > > > Using non-temporal stores can be up to 3x faster on ICX and 2x
> faster
> > >> > > > on SKX. Historically, these numbers would not have been so good
> > >> > > > because of the zero-over-zero writeback optimization that `rep
> stosb`
> > >> > > > is able to do. But, the zero-over-zero writeback optimization
> has been
> > >> > > > removed as a potential side-channel attack, so there is no
> longer any
> > >> > > > good reason to only rely on `rep stosb` for large memsets. On
> the flip
> > >> > > > size, non-temporal writes can avoid data in their RFO requests
> saving
> > >> > > > memory bandwidth.
> > >> > >
> > >> > > I'm actually working on RHEL-29312 that you reference in the
> subject
> > >> > > (thanks for the reference, but please don't reference downstream
> tickets
> > >> > > in upstream patches).  In that ticket was shown a 50% slowdown in
> memset
> > >> > > for the Xeon 4215.  I've reproduced that slowdown locally, but
> this
> > >> > > patch doesn't seem to have any affect on it (current git glibc
> still has
> > >> > > the slow code).  I can get the faster results with tunables[1].
> Will
> > >> > > there be further tuning of the default thresholds?  Or do we
> expect
> > >> > > users to use tunables to get around these regressions, for the
> specific
> > >> > > chips that are not optimized?
> > >> > >
> > >> > > [1] specifically, libc.cpu.x86_rep_stosb_threshold=0x1000000000 -
> none
> > >> > > of the others have any affect.
> > >> >
> > >> > Okay, I messed up. The SKX machine I used for the SKX benchmarks was
> > >> > mislabeled, and was in fact an ICX machine.
> > >> >
> > >> > I have some older SKX/BWD data:
> > >> >
> https://docs.google.com/spreadsheets/d/1rcRIWpU8Tcq0FMH9iGPBUViPvioUmHtA9qx3h-UzPa0/edit?usp=sharing
> > >> > and agree we need to increase the stosb threshold on SKX. I didn't
> see
> > >> > the huge (100MB threshold) you saw there, but am re-running
> benchmarks.
> > >> >
> > >> > An unfortunate note is BWD seems to prefer ERMS in the same range
> SKX
> > >> > doesn't, so we can't just make the decision based on the feature.
> > >> > On the bright side, it does look like the non-temporal decision
> (this patch)
> > >> > still makes sense on SKX.
> > >> >
> > >> > I am re-running benchmarks now and will see what we can do about
> raising
> > >> > the stosb threshold on SKX.
> > >> >
> > >> > I am deeply sorry for the confusion I have created here.
> > >>
> > >> I've re-ran some benchmarks and updated the google sheet:
> > >>
> https://docs.google.com/spreadsheets/d/1opzukzvum4n6-RUVHTGddV6RjAEil4P2uMjjQGLbLcU/edit?usp=sharing
> > >> See SKX/SKX-2.
> > >>
> > >> Regarding ERMS:
> > >> What I am measuring so far indicates we want temporal writes
> > >> when the majority of the set fits in L1. Past that ERMS becomes
> better,
> > >> especially as thread count increases.
> > >>
> > >> Regarding NT Stores:
> > >> non-temporal doesn't look good in these benchmarks (particularly
> SKX-2).
> > >> I am running some additional tests to see exactly what is going on.
> > >> I am going to post a patch to disable non-temporal stores on Skylake,
> > >
> > >
> > > Skylake clients have very different NT behaviors.
> > > Please make sure that NT isn't disabled
> > > on clients.
> >
> > I guess my thinking right now is it's better to ensure we don't
> introduce a
> > new regression. The NT store code is new to 2.40, so disabling it won't
> > cause any regressions.
> > >
> > >>
> > >> if that is considered too risky given the 2.40 release I think we
> should revert
> > >> this patch (and the follow up AMD ones) to avoid a regression on SKX
> > >> and try to get them into 2.41 instead with proper tuning.
> Patch posted.
> I disabled for SKX only (im convinced enough its a function of SKX's unique
> memory system).
> > >>
> > >>
> > >> > >
> > >>
> > > H.J.
>

 I would like to backport this patch to release branches.

Any comments or objections?

--Sunil