mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"Julian Vetter" <julian@outer-limits.org>,
	"Madhavan Srinivasan" <maddy@linux.ibm.com>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Naveen N Rao" <naveen@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc: Remove eieio in _memcpy_fromio
Date: Tue, 28 Jan 2025 17:50:09 +0100	[thread overview]
Message-ID: <9fc9a994-b6cf-4ad9-9cec-6777dedd216d@app.fastmail.com> (raw)
In-Reply-To: <b8496571-6c44-40ff-b03b-799fb1764447@csgroup.eu>

On Tue, Jan 28, 2025, at 16:34, Christophe Leroy wrote:
> Le 28/01/2025 à 16:24, Christophe Leroy a écrit :
>> Le 28/01/2025 à 16:07, Julian Vetter a écrit :
>>> With 'ppc' I was refering to 'include/asm-ppc/io.h'. But you're right,
>>> when going back a bit, in the 'include/asm-powerpc/io.h' there are two
>>> cases, one (eeh_memcpy_fromio) which does the the 'eieio', and a second,
>>> i.e., 'iSeries_memcpy_fromio' which does a byte-wise copy. But in the
>>> ppc code ('include/asm-ppc/io.h') there is a simple memcpy. I was
>>> referring to this one. But my description is not very clear. Sorry for 
>>> that.
>> 
>> But then is your change still valid ? Isn't there some corner case that 
>> still need it ? Is it a valid argument that because memcpy_toio() 
>> doesn't need it memcpy_fromio() doesn't need it either ?
>
> I see that _insb(), _insw_ns() and _insl_ns() also have eieio() while 
> _outsb(), _outsw_ns() and _outsl_ns() don't. Why not change those as 
> well if you think eieio() is not needed ?

I think that makes sense, even if it's beyond the scope of Julian's
work to unify the memcpy/memset I/O helpers across architectures.

I looked into the pre-2.6.12 history of arch/powerpc64 to see how
the eieio got in there originally and found that at the time the
string functions got added, this is what the readl() etc functions
did. readl() itself went through a longer set of changes to end
up with the current sync/twi/isync version, but the string functions
were never updated again during any of the later changes.

The bit that needs to be captured in the changelog here is that
on all other architectures, strcpy_fromio/strcpy_toio are written
to allow prefetching/combining/reordering, while the powerpc
version prevents this in strcpy_fromio for apparently only history
reasons.

     Arnd

      reply	other threads:[~2025-01-28 16:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28 13:57 Julian Vetter
2025-01-28 14:16 ` Christophe Leroy
2025-01-28 15:07   ` Julian Vetter
2025-01-28 15:24     ` Christophe Leroy
2025-01-28 15:34       ` Christophe Leroy
2025-01-28 16:50         ` Arnd Bergmann [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=9fc9a994-b6cf-4ad9-9cec-6777dedd216d@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=christophe.leroy@csgroup.eu \
    --cc=julian@outer-limits.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen@kernel.org \
    --cc=npiggin@gmail.com \
    /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

all inboxes | Powered by JetHome®