The Linux Kernel Mailing List
 help / color / mirror / Atom feed
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.


  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