Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Will Deacon <will.deacon@arm.com>,
	Ard.Biesheuvel@arm.com, Andrew Murray <andrew.murray@arm.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v1 0/5] arm64: avoid out-of-line ll/sc atomics
Date: Fri, 17 May 2019 14:19:13 +0200
Message-ID: <CAKv+Gu8390GxxxzQb4phoCeGhNaXpHBSen-mEz1HUp7OAc=5vw@mail.gmail.com> (raw)
In-Reply-To: <20190517120522.GM2623@hirez.programming.kicks-ass.net>

On Fri, 17 May 2019 at 14:05, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, May 17, 2019 at 11:08:03AM +0100, Andrew Murray wrote:
>
> > I think the alternative solution (excuse the pun) that you are suggesting
> > is to put the body of the ll/sc or LSE code in the ALTERNATIVE oldinstr/newinstr
> > blocks (i.e. drop the fallback branches). However this still gives us some
> > bloat (but less than my current solution) because we're still now inlining the
> > larger fallback ll/sc whereas previously they were non-inline'd functions. We
> > still end up with potentially unnecessary clobbers for LSE code with this
> > Approach prior to this series:
>
> > Approach using alternative without braces:
> >
> >    LSE
> >    LSE
> >    NOP
> >    NOP
> >
> > or
> >
> >    LL/SC <- inlined LL/SC and thus duplicated
> >    LL/SC
> >    LL/SC
> >    LL/SC
>
> Yes that. And if you worry about the extra clobber for LL/SC, you could
> always stuck a few PUSH/POPs around the LL/SC block.

Patching in pushes and pops replaces a potential performance hit in
the LSE code with a guaranteed performance hit in the LL/SC code, and
you may end up pushing and popping dead registers. So it would be nice
to see some justification for disproportionately penalizing the LL/SC
code (which will be used on low end cores where stack accesses are
relatively expensive) relative to the LSE code, rather than assuming
that relieving the register pressure on the current hot paths will
result in a measurable performance improvement on LSE systems.

>  Although I'm not
> exactly sure where the x16,x17,x30 clobbers come from; then I look at
> the LL/SC code, there aren't any hard-coded regs in there.
>

The out of line LL/SC code is invoked as a function call, and so we
need to preserve x30 which contains the return value.

x16 and x17 are used by the PLT branching code, in case the module
invoking the atomics is too far away from the core kernel for an
ordinary relative branch.

> Also, the safe approach is to emit LL/SC as the default and only patch
> in LSE when you know the machine supports them.
>

Given that it is not only the safe approach, but the only working
approach, we are obviously already doing that both in the old and the
new version of the code.

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

      reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16 15:53 Andrew Murray
2019-05-16 15:53 ` [PATCH v1 1/5] jump_label: Don't warn on __exit jump entries Andrew Murray
2019-05-16 15:53 ` [PATCH v1 2/5] arm64: Use correct ll/sc atomic constraints Andrew Murray
2019-05-16 15:53 ` [PATCH v1 3/5] arm64: atomics: avoid out-of-line ll/sc atomics Andrew Murray
2019-05-16 15:53 ` [PATCH v1 4/5] arm64: avoid using hard-coded registers for LSE atomics Andrew Murray
2019-05-16 15:53 ` [PATCH v1 5/5] arm64: atomics: remove atomic_ll_sc compilation unit Andrew Murray
2019-05-17  7:24 ` [PATCH v1 0/5] arm64: avoid out-of-line ll/sc atomics Peter Zijlstra
2019-05-17 10:08   ` Andrew Murray
2019-05-17 10:29     ` Ard Biesheuvel
2019-05-22 10:45       ` Andrew Murray
2019-05-22 11:44         ` Ard Biesheuvel
2019-05-22 15:36           ` Andrew Murray
2019-05-17 12:05     ` Peter Zijlstra
2019-05-17 12:19       ` Ard Biesheuvel [this message]

Reply instructions:

You may reply publically 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='CAKv+Gu8390GxxxzQb4phoCeGhNaXpHBSen-mEz1HUp7OAc=5vw@mail.gmail.com' \
    --to=ard.biesheuvel@linaro.org \
    --cc=Ard.Biesheuvel@arm.com \
    --cc=andrew.murray@arm.com \
    --cc=boqun.feng@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=peterz@infradead.org \
    --cc=will.deacon@arm.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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git