From: David Laight <david.laight.linux@gmail.com>
To: Heiko Carstens <hca@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Juergen Christ <jchrist@linux.ibm.com>,
linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org
Subject: Re: [PATCH v5 1/7] s390/percpu: Infrastructure for more efficient this_cpu operations
Date: Tue, 2 Jun 2026 15:36:52 +0100 [thread overview]
Message-ID: <20260602153652.6c2c02bc@pumpkin> (raw)
In-Reply-To: <20260602135436.10415A71-hca@linux.ibm.com>
On Tue, 2 Jun 2026 15:54:36 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:
> On Tue, Jun 02, 2026 at 02:32:09PM +0100, David Laight wrote:
> > On Mon, 1 Jun 2026 17:08:13 +0200
> > Heiko Carstens <hca@linux.ibm.com> wrote:
> > > It is: the check makes sure this is an AG instruction, which adds the
> > > percpu offset from lowcore - by checking that the displacement is
> > > correct, as well as that the base register is zero.
> > >
> > > There could be a different AG instruction within the inline assembly,
> > > for whatever reason.
> >
> > Do you actually even need to check the instruction?
> >
> > This sequence can only work for simple per-cpu accesses, so I don't
> > see a reason to let the specified register point anywhere other than the
> > base of the per-cpu data.
> >
> > That means the process switch code can just load the register with the
> > base of the per-cpu data for the new cpu.
> > If that happens before the 'AG' is executed it won't matter.
> >
> > The only reason would be to support non-offsettable memory accesses.
> > But it looks like the 'laag %r5,%r2,0(%r4)' in the example has an
> > offset (of zero).
> > Probably only stops you doing a direct access of an array.
> >
> > That would mean that needs_fixup goes in the bin and percpu_exit() becomes:
> > ...
> > reg = regs->percpu_register;
> > if (likely(!reg))
> > return;
> > lc->percpu_register = reg;
> > regs->gprs[reg] = lc->percpu_offset
> > }
> >
> > I guess I'm missing something?
>
> The percpu register (in the above example %r4) first contains the base address
> of a percpu variable. To get the actual percpu address of the variable the
> percpu_offset of the corresponding cpu has to be added to that address, which
> is what the AG instruction is doing.
I knew my brain was fading.
I'm sure it should be possible to get the linker to put the offset of
the variable from the base on the per-cpu data area into the laag instruction.
(Looks like it has a 20bit offset field.)
Although I've no idea how per-cpu data works in loadable modules.
That might mean you need lc->percpu_address as well as lc->percpu_offset.
(If it isn't there already.)
-- David
>
> What you propose would make a CPU's percpu_offset the address of any percpu
> variable, which most likely would result in a crash.
next prev parent reply other threads:[~2026-06-02 14:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-26 5:56 [PATCH v5 0/7] s390: Improve this_cpu operations Heiko Carstens
2026-05-26 5:56 ` [PATCH v5 1/7] s390/percpu: Infrastructure for more efficient " Heiko Carstens
2026-06-01 7:10 ` Alexander Gordeev
2026-06-01 13:27 ` Heiko Carstens
2026-06-01 14:49 ` Alexander Gordeev
2026-06-01 15:08 ` Heiko Carstens
2026-06-02 13:32 ` David Laight
2026-06-02 13:54 ` Heiko Carstens
2026-06-02 14:36 ` David Laight [this message]
2026-06-02 10:08 ` Alexander Gordeev
2026-05-26 5:56 ` [PATCH v5 2/7] s390/percpu: Add missing do { } while (0) constructs Heiko Carstens
2026-06-01 7:14 ` Alexander Gordeev
2026-05-26 5:56 ` [PATCH v5 3/7] s390/percpu: Use new percpu code section for arch_this_cpu_add() Heiko Carstens
2026-06-02 11:33 ` Alexander Gordeev
2026-05-26 5:56 ` [PATCH v5 4/7] s390/percpu: Use new percpu code section for arch_this_cpu_add_return() Heiko Carstens
2026-06-02 11:34 ` Alexander Gordeev
2026-05-26 5:57 ` [PATCH v5 5/7] s390/percpu: Use new percpu code section for arch_this_cpu_[and|or]() Heiko Carstens
2026-06-02 11:35 ` Alexander Gordeev
2026-05-26 5:57 ` [PATCH v5 6/7] s390/percpu: Provide arch_this_cpu_read() implementation Heiko Carstens
2026-06-02 11:40 ` Alexander Gordeev
2026-05-26 5:57 ` [PATCH v5 7/7] s390/percpu: Provide arch_this_cpu_write() implementation Heiko Carstens
2026-06-02 11:40 ` Alexander Gordeev
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=20260602153652.6c2c02bc@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=jchrist@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=svens@linux.ibm.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
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