linux-kernel - Re: [patch 0/4] cleanup - use _AC macro to define PAGE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20080204202857.GB16919@uranus.ravnborg.org>
Date:	Mon, 4 Feb 2008 21:28:57 +0100
From:	Sam Ravnborg <sam@...nborg.org>
To:	Mike Frysinger <vapier.adi@...il.com>
Cc:	Cyrill Gorcunov <gorcunov@...il.com>, linux-kernel@...r.kernel.org,
	davem@...emloft.net, bryan.wu@...log.com, hskinnemoen@...el.co,
	dhowells@...hat.com
Subject: Re: [patch 0/4] cleanup - use _AC macro to define PAGE_SIZE

On Mon, Feb 04, 2008 at 03:01:30PM -0500, Mike Frysinger wrote:
> On Feb 3, 2008 7:22 AM, Sam Ravnborg <sam@...nborg.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/