Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

[Sam Ravnborg <sam@ravnborg.org>]

On Mon, Feb 04, 2008 at 03:01:30PM -0500, Mike Frysinger wrote:
> On Feb 3, 2008 7:22 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
> > On Sun, Feb 03, 2008 at 02:22:17PM +0300, Cyrill Gorcunov wrote:
> > > This patch series removes code duplication for PAGE_SIZE
> > > definition on various architectures.
> > >
> > > Please review. Any comments are welcome. *UNTESTED*
> >
> > Looks good.
> > Could you do a followup patch for each architecture introducing
> > use of PAGE_SIZE in their respective kernel/vmlinux.lds.S too?
> > I think more than your 4 listed architectures could need this fix.
> 
> what did you have in mind exactly ?  afaik, the Blackfin linker script
> should already be friendly wrt using defines instead of hardcoded
> constants ...
Blackfin look ok.
Lets take following code snippet from blackfin:
       .init.text :
        {
                . = ALIGN(PAGE_SIZE);
                __sinittext = .;
                INIT_TEXT
                __einittext = .;
        }


And a grep quicly reveals:

m32r/kernel/vmlinux.lds.S-  . = ALIGN(4096);            /* Init code and data */
m32r/kernel/vmlinux.lds.S-  __init_begin = .;
m32r/kernel/vmlinux.lds.S-  .init.text : {
m32r/kernel/vmlinux.lds.S-      _sinittext = .;
m32r/kernel/vmlinux.lds.S:      INIT_TEXT
m32r/kernel/vmlinux.lds.S-      _einittext = .;
m32r/kernel/vmlinux.lds.S-  }
m32r/kernel/vmlinux.lds.S-  .init.data : { INIT_DATA }
m32r/kernel/vmlinux.lds.S-  . = ALIGN(16);
m32r/kernel/vmlinux.lds.S-  __setup_start = .;
--
m68knommu/kernel/vmlinux.lds.S-
m68knommu/kernel/vmlinux.lds.S- .init : {
m68knommu/kernel/vmlinux.lds.S-         . = ALIGN(4096);
m68knommu/kernel/vmlinux.lds.S-         __init_begin = .;
m68knommu/kernel/vmlinux.lds.S-         _sinittext = .;
m68knommu/kernel/vmlinux.lds.S:         INIT_TEXT
m68knommu/kernel/vmlinux.lds.S-         _einittext = .;

It is maybe common knowledge that 4096 is equal to PAGE_SIZE
but it is more descriptive to use a constant.

As for sparc64 where we fixed this recently it was a bug since the
page size are configuration dependent.

So I just ask to use appropriate constants when they are
already available and visible from the vmlinux.lds.S files.

Consistent indenting in all vmlinux.lds.S files would also
be good - but I am still missing a third of them.
Blackfin looks good also in this respect.

	Sam