linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: linux-arch@vger.kernel.org, Will Deacon <will.deacon@arm.com>
Cc: Andrea Parri <andrea.parri@amarulasolutions.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Rich Felker <dalias@libc.org>,
	David Howells <dhowells@redhat.com>,
	Daniel Lustig <dlustig@nvidia.com>,
	linux-kernel@vger.kernel.org,
	"Maciej W. Rozycki" <macro@linux-mips.org>,
	Ingo Molnar <mingo@kernel.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Palmer Dabbelt <palmer@sifive.com>,
	Paul Burton <paul.burton@mips.com>,
	"Paul E. McKenney" <paulmck@linux.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Tony Luck <tony.luck@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>
Subject: Re: [RFC PATCH 11/20] ia64: Add unconditional mmiowb() to arch_spin_unlock()
Date: Wed, 27 Feb 2019 14:36:05 +1000	[thread overview]
Message-ID: <1551241306.u5r150hwwb.astroid@bobo.none> (raw)
In-Reply-To: <20190222185026.10973-12-will.deacon@arm.com>

Will Deacon's on February 23, 2019 4:50 am:
> The mmiowb() macro is horribly difficult to use and drivers will continue
> to work most of the time if they omit a call when it is required.
> 
> Rather than rely on driver authors getting this right, push mmiowb() into
> arch_spin_unlock() for ia64. If this is deemed to be a performance issue,
> a subsequent optimisation could make use of ARCH_HAS_MMIOWB to elide
> the barrier in cases where no I/O writes were performned inside the
> critical section.

mmiowb() was always the wrong approach. IIRC what happened is that an
ia64 platform found that real wmb() semantics were too expensive, so 
they kind of "relaxed" it, breaking everything, and then said drivers
that wanted to unbreak themselves had to add these mmiowb() in.

The right way to go of course would have been to implement wmb()
the way existing drivers expected, and add a faster io_wmb() that
only ordered mmio stores from the CPU added to the few drivers that
the platform cared about.

I think it was argued the wmb() was still technically correct because
the reordering did not happen at the CPU, but somewhere else in the 
interconnect or PCI controller. But that was just a crazy burden to
put on driver writers, and it was why the documentation was always
incomprehensible.

Not sure why Linus ever went along with it, but awesome you're removing
it. Thank you!

Thanks,
Nick

  reply	other threads:[~2019-02-27  4:36 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22 18:50 [RFC PATCH 00/20] Remove Mysterious Macro Intended to Obscure Weird Behaviours (mmiowb()) Will Deacon
2019-02-22 18:50 ` [RFC PATCH 01/20] asm-generic/mmiowb: Add generic implementation of mmiowb() tracking Will Deacon
2019-02-22 21:49   ` Linus Torvalds
2019-02-22 21:55     ` Linus Torvalds
2019-02-26 18:25       ` Will Deacon
2019-02-26 18:26     ` Will Deacon
2019-02-26 18:33       ` Peter Zijlstra
2019-02-26 19:02         ` Linus Torvalds
2019-02-27 10:19           ` Peter Zijlstra
2019-02-22 18:50 ` [RFC PATCH 02/20] arch: Use asm-generic header for asm/mmiowb.h Will Deacon
2019-02-22 18:50 ` [RFC PATCH 03/20] mmiowb: Hook up mmiowb helpers to spinlocks and generic I/O accessors Will Deacon
2019-02-22 18:50 ` [RFC PATCH 04/20] ARM: io: Remove useless definition of mmiowb() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 05/20] arm64: " Will Deacon
2019-02-22 18:50 ` [RFC PATCH 06/20] x86: " Will Deacon
2019-02-22 18:50 ` [RFC PATCH 07/20] nds32: " Will Deacon
2019-02-22 18:50 ` [RFC PATCH 08/20] m68k: " Will Deacon
2019-02-25  9:25   ` Geert Uytterhoeven
2019-02-25 12:12     ` Will Deacon
2019-02-22 18:50 ` [RFC PATCH 09/20] sh: Add unconditional mmiowb() to arch_spin_unlock() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 10/20] mips: " Will Deacon
2019-02-22 18:50 ` [RFC PATCH 11/20] ia64: " Will Deacon
2019-02-27  4:36   ` Nicholas Piggin [this message]
2019-02-22 18:50 ` [RFC PATCH 12/20] powerpc: mmiowb: Hook up mmwiob() implementation to asm-generic code Will Deacon
2019-02-22 18:50 ` [RFC PATCH 13/20] riscv: " Will Deacon
2019-02-22 18:50 ` [RFC PATCH 14/20] Documentation: Kill all references to mmiowb() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 15/20] drivers: Remove useless trailing comments from mmiowb() invocations Will Deacon
2019-02-22 18:50 ` [RFC PATCH 16/20] drivers: Remove explicit invocations of mmiowb() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 17/20] scsi: qla1280: Remove stale comment about mmiowb() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 18/20] i40iw: Redefine i40iw_mmiowb() to do nothing Will Deacon
2019-02-22 18:50 ` [RFC PATCH 19/20] net: silan: sc92031: Remove stale comment about mmiowb() Will Deacon
2019-02-22 18:50 ` [RFC PATCH 20/20] arch: Remove dummy mmiowb() definitions from arch code 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=1551241306.u5r150hwwb.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=andrea.parri@amarulasolutions.com \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=dalias@libc.org \
    --cc=dhowells@redhat.com \
    --cc=dlustig@nvidia.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=macro@linux-mips.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=palmer@sifive.com \
    --cc=paul.burton@mips.com \
    --cc=paulmck@linux.ibm.com \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=ysato@users.sourceforge.jp \
    /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).