2009-10-15 05:33:55

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: powerpc problem with .data.page_aligned -> __page_aligned_data conversion

Hi folks !

So I have a problem ... :-)

For some weird reason, our gcc until 4.3 (fixed in 4.3) had the weird
idea that the alignment attribute should not be allowed to force an
alignment greater than 32k. If attempted, it would warn -and- crop the
alignment to 32k.

As you can imagine, this doesn't play terribly well with 64K page size,
which is (sadly) our default on ppc64 on distros nowadays. I know it's
insane but that's what I have to live with...

And of course, our current distros (well, at least RHEL5 which is pretty
common) don't have a fixed gcc.

Until recently, we got away with it because very few people used
__page_aligned and those who did always did it on something that is
naturally a PAGE_SIZE multiple in size. So we just stick the thing into
the appropriate section and that's it.

The new generic macros __page_aligned_data/bss now always add the
alignment directive generically.

This has a few issues for us:

- The patch that converted bits of powerpc to the new macro break since
it now hits that bug

- We can't trivially change the macros to not to the align directive on
powerpc, but that's taking the risk that drivers or other pieces of
generic code now assume that they can use __page_aligned_* on a data
structure whose size isn't a multiple of _PAGE_SIZE, since that would
work on all architectures ... except powerpc where it would silently
break by shifting everybody else alignment in that section.

As of today, nothing seems to do that (unless I mis-grepped) though.

What do you recommend I do ? I can ban gcc < 4.3 but that's a bit
harsh :-) I know a few people that won't be happy to be unable to build
newer kernels with current distro gccs.

Or can do the above making the macro definition drop the alignment part
on powerpc. Will work for now, but will require great care to avoid
subtle and nasty breakage (basically same as before)

Or maybe I can do the above but only when using gcc < 4.3 so at least if
the breakage happen, that will only be with older gccs ...

Unless somebody has a better idea ?

Cheers,
Ben.


2009-10-15 16:39:20

by Tim Abbott

[permalink] [raw]
Subject: Re: powerpc problem with .data.page_aligned -> __page_aligned_data conversion

On Thu, 15 Oct 2009, Benjamin Herrenschmidt wrote:

> For some weird reason, our gcc until 4.3 (fixed in 4.3) had the weird
> idea that the alignment attribute should not be allowed to force an
> alignment greater than 32k. If attempted, it would warn -and- crop the
> alignment to 32k.
[...]
> This has a few issues for us:
>
> - The patch that converted bits of powerpc to the new macro break since
> it now hits that bug

Hi Ben,

Just to make sure I understand the nature of the problem, is the current
breakage that gcc < 4.3 will _warn_ on any compilation units on ppc64 that
use __page_aligned data, or something worse?

The cropping is clearly a potential problem, but I read the rest of your
email as saying that the cropping of the alignment isn't actually a
problem with the current kernel because the kernel is currently only using
the macro with things whose size is divisible by PAGE_SIZE. However, I am
not sure how to reconcile that with using the word "break" above...

-Tim Abbott

2009-10-15 16:47:37

by Tim Abbott

[permalink] [raw]
Subject: Re: powerpc problem with .data.page_aligned -> __page_aligned_data conversion

On Thu, 15 Oct 2009, Benjamin Herrenschmidt wrote:

> What do you recommend I do ?
> I can ban gcc < 4.3 but that's a bit harsh :-)

Yeah, let's try to avoid that.

> I know a few people that won't be happy to be unable to build newer
> kernels with current distro gccs.
>
> Or can do the above making the macro definition drop the alignment part
> on powerpc. Will work for now, but will require great care to avoid
> subtle and nasty breakage (basically same as before)

Yeah, I'd be afraid that changing the generic __page_aligned_data might
cause unexpected problems on some other architecture.

> Or maybe I can do the above but only when using gcc < 4.3 so at least if
> the breakage happen, that will only be with older gccs ...

It sounds like from your grepping, you don't believe that dropping the
alignment part will actually cause any problems on powerpc currently?

If so, dropping the alignment part on powerpc with gcc < 4.3 seems best to
me. It limits the workaround in time (eventually gcc < 4.3 will be
history). It also limits it in scope (to powerpc), where at least you're
well aware of the issue and can pay attention to new code being added that
uses __page_aligned_data. Since most code that has page-aligned data
structures is architecture-specific, there's a good chance that any new
code that would break will be at least looked at by you (and given how few
places it is used currently, this seems pretty unlikely to actually come
up).

-Tim Abbott

2009-10-15 20:09:23

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: powerpc problem with .data.page_aligned -> __page_aligned_data conversion

On Thu, 2009-10-15 at 12:37 -0400, Tim Abbott wrote:
> Just to make sure I understand the nature of the problem, is the
> current
> breakage that gcc < 4.3 will _warn_ on any compilation units on ppc64
> that
> use __page_aligned data, or something worse?
>
> The cropping is clearly a potential problem, but I read the rest of
> your
> email as saying that the cropping of the alignment isn't actually a
> problem with the current kernel because the kernel is currently only
> using
> the macro with things whose size is divisible by PAGE_SIZE. However,
> I am
> not sure how to reconcile that with using the word "break" above...

Break is because we use -Werror on arch/powerpc :-)

Ben.