linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: paulmck <paulmck@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Jann Horn <jannh@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
	stable <stable@vger.kernel.org>, Will Deacon <will@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Andy Lutomirski <luto@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode()
Date: Wed, 30 Dec 2020 21:57:21 +1000	[thread overview]
Message-ID: <1609327110.c18a3h158t.astroid@bobo.none> (raw)
In-Reply-To: <20201230105847.GQ1551@shell.armlinux.org.uk>

Excerpts from Russell King - ARM Linux admin's message of December 30, 2020 8:58 pm:
> On Wed, Dec 30, 2020 at 10:00:28AM +0000, Russell King - ARM Linux admin wrote:
>> On Wed, Dec 30, 2020 at 12:33:02PM +1000, Nicholas Piggin wrote:
>> > Excerpts from Russell King - ARM Linux admin's message of December 29, 2020 8:44 pm:
>> > > On Tue, Dec 29, 2020 at 01:09:12PM +1000, Nicholas Piggin wrote:
>> > >> I think it should certainly be documented in terms of what guarantees
>> > >> it provides to application, _not_ the kinds of instructions it may or
>> > >> may not induce the core to execute. And if existing API can't be
>> > >> re-documented sanely, then deprecatd and new ones added that DTRT.
>> > >> Possibly under a new system call, if arch's like ARM want a range
>> > >> flush and we don't want to expand the multiplexing behaviour of
>> > >> membarrier even more (sigh).
>> > > 
>> > > The 32-bit ARM sys_cacheflush() is there only to support self-modifying
>> > > code, and takes whatever actions are necessary to support that.
>> > > Exactly what actions it takes are cache implementation specific, and
>> > > should be of no concern to the caller, but the underlying thing is...
>> > > it's to support self-modifying code.
>> > 
>> >    Caveat
>> >        cacheflush()  should  not  be used in programs intended to be portable.
>> >        On Linux, this call first appeared on the MIPS architecture, but  nowa‐
>> >        days, Linux provides a cacheflush() system call on some other architec‐
>> >        tures, but with different arguments.
>> > 
>> > What a disaster. Another badly designed interface, although it didn't 
>> > originate in Linux it sounds like we weren't to be outdone so
>> > we messed it up even worse.
>> > 
>> > flushing caches is neither necessary nor sufficient for code modification
>> > on many processors. Maybe some old MIPS specific private thing was fine,
>> > but certainly before it grew to other architectures, somebody should 
>> > have thought for more than 2 minutes about it. Sigh.
>> 
>> WARNING: You are bordering on being objectionable and offensive with
>> that comment.
>> 
>> The ARM interface was designed by me back in the very early days of
>> Linux, probably while you were still in dypers, based on what was
>> known at the time.  Back in the early 2000s, ideas such as relaxed
>> memory ordering were not known.  All there was was one level of
>> harvard cache.

I wasn't talking about memory ordering at all, and I assumed it
came earlier and was brought to Linux for portability reasons -

CONFORMING TO
       Historically, this system call was available on all MIPS UNIX  variants
       including RISC/os, IRIX, Ultrix, NetBSD, OpenBSD, and FreeBSD (and also
       on some non-UNIX MIPS operating systems), so that the existence of this
       call in MIPS operating systems is a de-facto standard.

I don't think the call was totally unreasonable for incoherent virtual 
caches or incoherent i/d caches etc. Although early unix system call interface
demonstrates that people understood how to define good interfaces that dealt
with intent at an abstract level rather than implementation -- munmap 
doesn't specify anywhere that a TLB flush instruction must be executed, 
for example. So "cacheflush" was obviously never a well designed interface 
but rather the typical hardware-centric hack to get their stuff working
(which was fine for its purpose I'm sure).

> 
> Sorry, I got that slightly wrong. Its origins on ARM were from 12 Dec
> 1998:
> 
> http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=88/1
> 
> by Philip Blundell, and first appeared in the ARM
> pre-patch-2.1.131-19981214-1.gz. It was subsequently documented in the
> kernel sources by me in July 2001 in ARM patch-2.4.6-rmk2.gz. It has
> a slightly different signature: the third argument on ARM is a flags
> argument, whereas the MIPS code, it is some undocumented "cache"
> parameter.
> 
> Whether the ARM version pre or post dates the MIPS code, I couldn't say.
> Whether it was ultimately taken from the MIPS implementation, again, I
> couldn't say.

I can, it was in MIPS in late 1.3 kernels at least. I would guess it
came from IRIX.

> However, please stop insulting your fellow developers ability to think.

Sorry, I was being melodramatic. Everyone makes mistakes or decisions
which with hindsight could have been better or were under some 
constraint that isn't apparent. I shouldn't have suggested it indicated 
thoughtlessness.

Thanks,
Nick

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-12-30 11:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-27 18:28 [RFC please help] membarrier: Rewrite sync_core_before_usermode() Andy Lutomirski
     [not found] ` <1836294649.3345.1609100294833.JavaMail.zimbra@efficios.com>
2020-12-27 21:36   ` Andy Lutomirski
2020-12-28 10:25     ` Russell King - ARM Linux admin
2020-12-28 17:14       ` Andy Lutomirski
2020-12-28 17:23         ` Russell King - ARM Linux admin
2020-12-28 18:10           ` Andy Lutomirski
2020-12-28 18:29         ` Jann Horn
2020-12-28 18:50           ` Andy Lutomirski
2020-12-28 19:08           ` Russell King - ARM Linux admin
2020-12-28 19:44             ` Andy Lutomirski
2020-12-28 20:24               ` Russell King - ARM Linux admin
     [not found]               ` <1086654515.3607.1609187556216.JavaMail.zimbra@efficios.com>
2020-12-28 21:06                 ` Andy Lutomirski
2020-12-29  0:36                   ` Nicholas Piggin
2020-12-29  0:56                     ` Andy Lutomirski
2020-12-29  3:09                       ` Nicholas Piggin
2020-12-29 10:44                         ` Russell King - ARM Linux admin
2020-12-30  2:33                           ` Nicholas Piggin
2020-12-30 10:00                             ` Russell King - ARM Linux admin
2020-12-30 10:58                               ` Russell King - ARM Linux admin
2020-12-30 11:57                                 ` Nicholas Piggin [this message]
     [not found]     ` <1670059472.3671.1609189779376.JavaMail.zimbra@efficios.com>
2020-12-29  0:30       ` Andy Lutomirski
2020-12-29  0:11 ` Nicholas Piggin
2020-12-29  0:36   ` Andy Lutomirski
2020-12-29  3:31     ` Nicholas Piggin
2021-01-01 18:33     ` David Laight
2021-01-05 13:26     ` Will Deacon
2021-01-05 16:20       ` Andy Lutomirski
2021-01-05 16:37         ` Peter Zijlstra
2021-01-05 22:41         ` Will Deacon

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=1609327110.c18a3h158t.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=jannh@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulmck@kernel.org \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.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).