Re: [PATCH v5] alpha: fix memory barriers so that they conform to the specification

From: Arnd Bergmann <arnd@arndb.de>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: "Maciej W. Rozycki" <macro@wdc.com>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	"Maciej W. Rozycki" <macro@linux-mips.org>,
	Richard Henderson <rth@twiddle.net>,
	Matt Turner <mattst88@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	alpha <linux-alpha@vger.kernel.org>,
	linux-serial@vger.kernel.org, linux-rtc@vger.kernel.org
Subject: Re: [PATCH v5] alpha: fix memory barriers so that they conform to the specification
Date: Mon, 25 May 2020 16:07:17 +0200	[thread overview]
Message-ID: <CAK8P3a0GVNvM3Sjcujwc0ybfxSftE9PfFBuXdUm1imfpQHmyig@mail.gmail.com> (raw)
In-Reply-To: <alpine.LRH.2.02.2005250944210.26265@file01.intranet.prod.int.rdu2.redhat.com>

On Mon, May 25, 2020 at 3:56 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Sun, 24 May 2020, Maciej W. Rozycki wrote:

> >
> > respectively, with corresponding out-of-line entry points available, so
> > that there is no extra inline code produced where the call to the relevant
> > MMIO accessor is going to end up with an actual function call, as this
> > would not help performance in any way and would expand code unnecessarily
> > at all call sites.
> >
> >  Therefore I suggest that your new `static inline' functions follow the
> > pattern, perhaps by grouping them with the corresponding ordinary accessor
> > functions in arch/alpha/include/asm/io.h within the relevant existing
> > #ifdef, and then by making them `extern inline' and providing out-of-line
> > implementations in arch/alpha/kernel/io.c, with the individual symbols
> > exported.  Within arch/alpha/kernel/io.c the compiler will still inline
> > code as it sees fit as it already does, e.g. `__raw_readq' might get
> > inlined in `readq' if it turns out cheaper than arranging for an actual
> > call, including all the stack frame preparation for `ra' preservation;
> > it's less likely with say `writeq' which probably always ends with a tail
> > call to `__raw_writeq' as no stack frame is required in that case.
> >
> >  That for the read accessors.
>
> I think that making the read*_relaxed functions extern inline just causes
> source code bloat with no practical gain - if we make them extern inline,
> we would need two implementations (one in the include file, the other in
> the C file) - and it is not good practice to duplicate code.
>
> The functions __raw_read* are already extern inline, so the compiler will
> inline/noinline them depending on the macros trivial_io_bw and
> trivial_io_lq - so we can just call them from read*_relaxed without
> repeating the extern inline pattern.

You could consider using the helpers in include/asm-generic/io.h
to provide some of the wrappers and only provide the ones that
don't fit in that scheme already.

       Arnd