Re: [PATCH] Fix initramfs size calculation

Previous thread: [PATCH 10/30] block: implement REQ_FLUSH/FUA based interface for FLUSH/FUA requests by Tejun Heo on Wednesday, August 25, 2010 - 8:47 am. (1 message)

Next thread: [PATCH 11/30] block: filter flush bio's in __generic_make_request() by Tejun Heo on Wednesday, August 25, 2010 - 8:47 am. (1 message)
From: Michael Holzheu
Date: Wednesday, August 25, 2010 - 8:57 am

The size of a built-in initramfs is calculated in init/initramfs.c by 
"__initramfs_end - __initramfs_start".  Those symbols are defined in the
linker script include/asm-generic/vmlinux.lds.h:

#define INIT_RAM_FS                                                     \
        . = ALIGN(PAGE_SIZE);                                           \
        VMLINUX_SYMBOL(__initramfs_start) = .;                          \
        *(.init.ramfs)                                                  \
        VMLINUX_SYMBOL(__initramfs_end) = .;

If the initramfs file has an odd number of bytes, the "__initramfs_end"
symbol points to an odd address, for example, the symbols in the System.map
might look like:

    0000000000572000 T __initramfs_start
    00000000005bcd05 T __initramfs_end	  <-- odd address

At least on s390 this is a problem:

Certain s390 instructions, especially instructions for loading addresses
(larl) or branch addresses must be on even addresses. 
The compiler loads the symbol addresses with the "larl". This instruction sets the
last bit to 0, therefore, for odd size files, the calculated size is one byte
less than it should be:

    0000000000540a9c <populate_rootfs>:
      540a9c:     eb cf f0 78 00 24       stmg    %r12,%r15,120(%r15),
      540aa2:     c0 10 00 01 8a af       larl    %r1,572000 <__initramfs_start>
      540aa8:     c0 c0 00 03 e1 2e       larl    %r12,5bcd04 <initramfs_end>
                                                  (Instead of  5bcd05)
      ...
      540abe:     1b c1                   sr      %r12,%r1

To fix the problem, this patch introduces the global variable
__initramfs_size, which is calculated in the "usr/initramfs_data.xxx.S" files.
The populate_rootfs() function can then use the init.ramfs section start and
the value of __initramfs_size for loading the initramfs.

The patch also restructures the "usr/initramfs_data.xxx.S" files to use a
common macro that includes the (compressed) initramfs file and calculates
the ...
From: Sam Ravnborg
Date: Wednesday, August 25, 2010 - 11:06 am

Another way to fix this could be to align . to an even

32 was selected as this is what we will introduce as the default
alignment in linker scripts anyway.

This I guess is a problem we have had some time and a minimal fix is
I like this anyway. Could you do this as a separate patch?

	Sam
--

From: Hendrik Brueckner
Date: Wednesday, August 25, 2010 - 12:15 pm

Sam,

I have work together with Michael on this patch... see my comments below


The first thought was similar but using ALIGN(2).  However, the current
implementation of populate_rootfs() passes the calculated size to the
decompress functions.  If __initramfs_end is aligned, the resulting size
might be greater than the real size of the initramfs.

I think that Michael or me could do this.


Kind regards,
Hendrik
--

From: Sam Ravnborg
Date: Wednesday, August 25, 2010 - 12:56 pm

Another variant could be to explicit add alignment in the .S files
so the section is always aligned to a even byte boundary (or maybe 32 byte boundary).

That should also cover the above case.

	Sam
--

From: Michael Holzheu
Date: Thursday, August 26, 2010 - 1:55 am

Hello Sam,


Then still the unpack_to_rootfs() function will not get the correct size
of the compressed file, because "__initramfs_end - __initramfs_start"
can be bigger than the real file size depending on the used the
alignment. I would assume that not all decompression functions will
work, if a bigger value is passed to decompress_method().

Michael




--

From: H. Peter Anvin
Date: Wednesday, August 25, 2010 - 1:10 pm

I have to admit to finding this fairly disturbing, as this is likely to
come up again and again.  Is there any way to tell gcc (e.g. with an
alignment attribute) that a particular symbol is not safe to be loaded
with larl?

	-hpa
--

From: Michael Holzheu
Date: Thursday, August 26, 2010 - 1:46 am

Hello Peter,


I think, it is not possible the change that in the compiler. The fact
that the compiler on s390 assumes that symbols are 2 byte aligned was a
design decission and is part of the ABI.

Michael 

--

Previous thread: [PATCH 10/30] block: implement REQ_FLUSH/FUA based interface for FLUSH/FUA requests by Tejun Heo on Wednesday, August 25, 2010 - 8:47 am. (1 message)

Next thread: [PATCH 11/30] block: filter flush bio's in __generic_make_request() by Tejun Heo on Wednesday, August 25, 2010 - 8:47 am. (1 message)