Re: [PATCH v5 1/7] s390/percpu: Infrastructure for more efficient this_cpu operations
From: Heiko Carstens
Date: Mon Jun 01 2026 - 11:17:02 EST
On Mon, Jun 01, 2026 at 04:49:35PM +0200, Alexander Gordeev wrote:
> On Mon, Jun 01, 2026 at 03:27:50PM +0200, Heiko Carstens wrote:
> ...
> > > > + if ((insn & 0xff0f) != 0xe300)
> > >
> > > Any reason mask/not to check the register number as well?
> >
> > Just to save code. If there would be a mismatch with the percpu
> > register, something serious would be wrong..
>
> Is it worth checking the disp then? ;)
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.
> > > > + * Inline assemblies making use of this typically have a code sequence like:
> > > > + *
> > > > + * MVIY_PERCPU(...) <- start of percpu code section
> > > > + * AG_ALT(...) <- add percpu offset; must be the second instruction
> > > > + * atomic_op <- atomic op
> > > \t here, but should be spaces?
> >
> > I can't follow? We have tabs in comments all over the place in s390 code.
>
> The other '<-' comments below and above use spaces, but this one
> mixes spaces with '\t'.
Because it is not possible to use tabs there. We put tabs in our
comments whenever possible.
> > > Probably it worth noting that no extra instructions between atomic_op
> > > and MVIY_ALT may exist (e.g. in the future), especially ones that use
> > > the percpu_register.
> >
> > That's not true. There may be additional instructions, they may just
> > not use the same register that is used for the percpu variable.
>
> That is what I tried to say, but I also thought it is intentionally
> only two instructions between mviy brackets allowed: ag + atomic_op.
> Am I wrong here?
Yes, you are wrong. You can use as many instructions as you want, as
long as the inline assembly follows the rules with respect to the
register which contains the percpu variable.
But seriously: all of this works only with atomic ops, so I don't see
reason why one put more instructions there. We actively try to keep
our inline assemblies as small as possible.
Even though with relocatable lowcore and alternatives the C code looks
huge, while it only generates very few instructions.
> > But that was true before this patch as well, even though it might not
> > have been "obvious".
>
> Hmm.. I do not get it. We would not get scheduled away before this patch,
> aren't we?
True. What I tried to say: before and after this patch there was and
is no code which _uses_ the pointer of the percpu variable more than
once. Accessing the percpu variable twice within such a section would
be potentially broken, since between two accesses an interrupt / nmi
could happen, which could modify the percpu var contents, which again
could result in loss of information.