Re: [GIT RFC] percpu: use dynamic percpu allocator as the default percpu allocator

Martin Schwidefsky
2009-03-25T11:28:01Z

On Wed, 25 Mar 2009 00:22:44 +0900
Tejun Heo <tj@kernel.org> wrote:

> Tejun Heo wrote:
> > Hello, Martin.
> >
> > Sorry about the delay.

Yes, dito..

> > Martin Schwidefsky wrote:
> >> We do have a problem with #2, the dynamic percpu patches currently
> >> breaks s390. But the nice thing is that we can now get rid of the GOTENT
> >> relocation for the percpu symbols. If the code is changed to use
> >> RELOC_HIDE for the SHIFT_PERCPU_PTR define, everything works just fine.
> >> Patch attached. Nice works guys.
> >>
> >> Subject: [PATCH] s390: percpu access.
> >>
> >> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> >>
> >> With the dynamic percpu allocator there is no need anymore to play
> >> tricks with the GOTENT relocation for the access to the percpu
> >> symbols. A simple RELOC_HIDE gets the job done.
> >
> > Hmm... I don't quite get it. The GOTENT was to work around large
> > offsets for modules, right? Can you please explain what changed by
> > the dynamic percpu allocator?

Unfortunately it didn't change. The problem is still there, only with
my particular configuration (and the correct patch) the system did
work because the problematic modules were not in use. But in general it
won't work.

The reason for the GOTENT indirection are static per-cpu variables that
are defined inside a module. The compiler considers these to be local.
For locally defined per_cpu__#var symbols the compiler uses an
instruction that is limited to the scope of a single object, which is
+-4 GB. The trick with GOTENT introduced an indirection which hid the
problem.

Without the GOTENT indirection the access to a static per cpu variable
will look like this:

0000000000000000 <test_fn>:
0: e3 30 03 30 00 04 lg %r3,816
6: c0 10 00 00 00 00 larl %r1,6 <test_fn+0x6>
8: R_390_PC32DBL .data.percpu+0x2
c: e3 23 10 00 00 04 lg %r2,0(%r3,%r1)

The R_390_PC32DBL relocation in the module relocation will fail if the
per-cpu area is farther than 4GB away from the vmalloc area.

With your patches and a RELOC_HIDE version that uses the GOTENT
indirection the kernel won't compile because the "X" constraint for
the GOTENT access needs a symbol and there are quite a few users that
pass a pointer. I do not see a simple solution for that problem yet.

> >> +#define SHIFT_PERCPU_PTR(var, offset) RELOC_HIDE(&per_cpu_var(var), (offset))
> >
> > Hmm... @var already has per_cpu__ prefix when the above macro is
> > invoked, so doing per_cpu_var() on it again wouldn't work. If simple
> > RELOC_HIDE works, you should be able to simply drop the above
> > definition. The generic percpu.h will define it.

That is true, in my working version of the oatch the second
per_cpu_var() wasn't there.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.