Message-Id: <1609327110.c18a3h158t.astroid@bobo.none>
Date:   Wed, 30 Dec 2020 21:57:21 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Jann Horn <jannh@...gle.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Andy Lutomirski <luto@...nel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        paulmck <paulmck@...nel.org>, Paul Mackerras <paulus@...ba.org>,
        Peter Zijlstra <peterz@...radead.org>,
        stable <stable@...r.kernel.org>, Will Deacon <will@...nel.org>,
        x86 <x86@...nel.org>
Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode()

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

Powered by blists - more mailing lists