From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: prefetch inconsistency in copy_page()
Date: Tue, 11 Jul 2017 19:26:00 +0100 [thread overview]
Message-ID: <20170711182559.GA14915@arm.com> (raw)
In-Reply-To: <CAKv+Gu9C7dn2cJ9nj0xyMRqn8yfqGj05hxONr1sOXuOUKaN_AA@mail.gmail.com>
On Tue, Jul 11, 2017 at 06:58:54PM +0100, Ard Biesheuvel wrote:
> On 11 July 2017 at 18:53, Pinski, Andrew <Andrew.Pinski@cavium.com> wrote:
> >> -----Original Message-----
> >> From: Will Deacon [mailto:will.deacon at arm.com]
> >> Sent: Tuesday, July 11, 2017 10:53 AM
> >> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Cc: Mark Rutland <mark.rutland@arm.com>; Pinski, Andrew
> >> <Andrew.Pinski@cavium.com>; linux-arm-kernel at lists.infradead.org
> >> Subject: Re: prefetch inconsistency in copy_page()
> >>
> >> On Tue, Jul 11, 2017 at 10:42:19AM +0100, Ard Biesheuvel wrote:
> >> > Hi all,
> >> >
> >> > No big deal, but I spotted an inconsistency in how copy_page() does
> >> > its prefetching. With the code as-is, it prefetches 2 lines at the
> >> > start of the function, but it is off by one line in the loop, i.e., it
> >> > prefetches 3 lines ahead (and skips a line at entry). So imo, we need
> >> > either
> >> >
> >> > """
> >> > @@ -30,9 +30,10 @@
> >> > */
> >> > ENTRY(copy_page)
> >> > alternative_if ARM64_HAS_NO_HW_PREFETCH
> >> > - # Prefetch two cache lines ahead.
> >> > + # Prefetch three cache lines ahead.
> >> > prfm pldl1strm, [x1, #128]
> >> > prfm pldl1strm, [x1, #256]
> >> > + prfm pldl1strm, [x1, #384]
> >> > alternative_else_nop_endif
> >> >
> >> > ldp x2, x3, [x1]
> >> > """
> >> >
> >> > or
> >> >
> >> > """
> >> > @@ -50,7 +51,7 @@ alternative_else_nop_endif
> >> > subs x18, x18, #128
> >> >
> >> > alternative_if ARM64_HAS_NO_HW_PREFETCH
> >> > - prfm pldl1strm, [x1, #384]
> >> > + prfm pldl1strm, [x1, #256]
> >> > alternative_else_nop_endif
> >> >
> >> > stnp x2, x3, [x0]
> >> >
> >> > """
> >> >
> >> > to make things consistent again.
> >> >
> >> > Thoughts?
> >>
> >> Either of those look good to me. Andrew -- any preference? If not, I'll
> >> take the second one.
> >
> > The first one is my preference but both are ok to me.
> >
>
> I suppose the second one is the least likely to invalidate existing
> benchmark results.
>
> Will: should I spin it as a proper patch? Mind if I clean up the
> whitespace at the same time?
Fill your boots!
Will
prev parent reply other threads:[~2017-07-11 18:26 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-11 9:42 prefetch inconsistency in copy_page() Ard Biesheuvel
2017-07-11 17:52 ` Will Deacon
2017-07-11 17:53 ` Pinski, Andrew
2017-07-11 17:58 ` Ard Biesheuvel
2017-07-11 18:26 ` Will Deacon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170711182559.GA14915@arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).