| Message ID | 20170814105231.14608-1-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | add support for relative references in special sections | expand |
On (08/14/17 11:52), Ard Biesheuvel wrote: > This adds support for emitting special sections such as initcall arrays, > PCI fixups and tracepoints as relative references rather than absolute > references. This reduces the size by 50% on 64-bit architectures, but > more importantly, it removes the need for carrying relocation metadata > for these sections in relocatables kernels (e.g., for KASLR) that need > to fix up these absolute references at boot time. On arm64, this reduces > the vmlinux footprint of such a reference by 8x (8 byte absolute reference > + 24 byte RELA entry vs 4 byte relative reference) [..] a side note, checkpatch complaints quite a lot. -ss
Hi Sergey, Thanks for taking a look On 18 August 2017 at 06:56, Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote: > On (08/14/17 11:52), Ard Biesheuvel wrote: >> This adds support for emitting special sections such as initcall arrays, >> PCI fixups and tracepoints as relative references rather than absolute >> references. This reduces the size by 50% on 64-bit architectures, but >> more importantly, it removes the need for carrying relocation metadata >> for these sections in relocatables kernels (e.g., for KASLR) that need >> to fix up these absolute references at boot time. On arm64, this reduces >> the vmlinux footprint of such a reference by 8x (8 byte absolute reference >> + 24 byte RELA entry vs 4 byte relative reference) > [..] > > a side note, > checkpatch complaints quite a lot. > Yeah, fair point. Many of them are debatable or completely bogus, though: ERROR: "foo * bar" should be "foo *bar" #114: FILE: include/linux/compiler.h:600: + static void * __attribute__((section(".discard.text"), used)) \ I think it is rather common to keep whitespace between * and what follows if it is not the identifier. ERROR: Macros with complex values should be enclosed in parentheses #147: FILE: include/linux/export.h:64: +#define __KSYMTAB_ENTRY(sym, sec) \ + __ADDRESSABLE(sym) \ + asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ + " .balign 8 \n" \ + VMLINUX_SYMBOL_STR(__ksymtab_##sym) ": \n" \ + " .long " VMLINUX_SYMBOL_STR(sym) "- . \n" \ + " .long " VMLINUX_SYMBOL_STR(__kstrtab_##sym) "- .\n" \ + " .previous \n") WARNING: do not add new typedefs #29: FILE: include/linux/init.h:114: +typedef signed int initcall_entry_t; WARNING: do not add new typedefs #36: FILE: include/linux/init.h:121: +typedef initcall_t initcall_entry_t; This is a typedef that accompanies the existing typedef for initcall_t, and it is the only way to parameterise this code without a ton of changes to duplicate all extern declarations of initcall arrays. ERROR: Macros with multiple statements should be enclosed in a do - while loop #55: FILE: include/linux/init.h:181: +#define ___define_initcall(fn, id, __sec) \ + __ADDRESSABLE(fn) \ + asm(".section \"" #__sec ".init\", \"a\" \n" \ + "__initcall_" #fn #id ": \n" \ + ".long " VMLINUX_SYMBOL_STR(fn) " - . \n" \ + ".previous \n"); This one is bogus. This macro is only used in file scope. WARNING: externs should be avoided in .c files #109: FILE: init/main.c:837: +extern initcall_entry_t __initcall0_start[]; This changes the type of the existing initcall array declarations, and I don't think moving them to a .h file for no other reason than to appease checkpatch is pointless. WARNING: unnecessary whitespace before a quoted newline #63: FILE: include/linux/pci.h:1759: + asm(".section " #sec ", \"a\" \n" \ WARNING: unnecessary whitespace before a quoted newline #64: FILE: include/linux/pci.h:1760: + ".balign 16 \n" \ WARNING: unnecessary whitespace before a quoted newline #65: FILE: include/linux/pci.h:1761: + ".short " #vendor ", " #device " \n" \ These are not newlines that end up as strings in the program, and I think it makes little sense to make the inline asm less readable by putting the \n right after the text. WARNING: braces {} are not necessary for single statement blocks #83: FILE: kernel/tracepoint.c:515: + for (iter = begin; iter < (signed int *)end; iter++) { + fct((struct tracepoint *)((unsigned long)iter + *iter), priv); + } This simply mirrors the existing code in the other branch of the if () I will clean up the meaningful ones in v2, but please don't expect this series to be checkpatch clean: it simply doesn't deal with inline asm very well, and some of this code predates checkpatch by a decade, and I'd rather not mix up rather tricky functional changes with checkpatch cleanup duty. -- Ard.
Hi Ard, On (08/18/17 07:12), Ard Biesheuvel wrote: > Hi Sergey, > > Thanks for taking a look > > On 18 August 2017 at 06:56, Sergey Senozhatsky > <sergey.senozhatsky.work@gmail.com> wrote: > > On (08/14/17 11:52), Ard Biesheuvel wrote: > >> This adds support for emitting special sections such as initcall arrays, > >> PCI fixups and tracepoints as relative references rather than absolute > >> references. This reduces the size by 50% on 64-bit architectures, but > >> more importantly, it removes the need for carrying relocation metadata > >> for these sections in relocatables kernels (e.g., for KASLR) that need > >> to fix up these absolute references at boot time. On arm64, this reduces > >> the vmlinux footprint of such a reference by 8x (8 byte absolute reference > >> + 24 byte RELA entry vs 4 byte relative reference) > > [..] > > > > a side note, > > checkpatch complaints quite a lot. > > [..] > I will clean up the meaningful ones in v2, but please don't expect > this series to be checkpatch clean: it simply doesn't deal with inline > asm very well, and some of this code predates checkpatch by a decade, > and I'd rather not mix up rather tricky functional changes with > checkpatch cleanup duty. sure. thanks. I'm running two x86 boxes with the patch set applied, for several days, with no issues being observed. it does save some memory (well, several pages in my case) even on "tiny" kernels configs. -ss
On 18 August 2017 at 07:29, Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote: > Hi Ard, > > On (08/18/17 07:12), Ard Biesheuvel wrote: >> Hi Sergey, >> >> Thanks for taking a look >> >> On 18 August 2017 at 06:56, Sergey Senozhatsky >> <sergey.senozhatsky.work@gmail.com> wrote: >> > On (08/14/17 11:52), Ard Biesheuvel wrote: >> >> This adds support for emitting special sections such as initcall arrays, >> >> PCI fixups and tracepoints as relative references rather than absolute >> >> references. This reduces the size by 50% on 64-bit architectures, but >> >> more importantly, it removes the need for carrying relocation metadata >> >> for these sections in relocatables kernels (e.g., for KASLR) that need >> >> to fix up these absolute references at boot time. On arm64, this reduces >> >> the vmlinux footprint of such a reference by 8x (8 byte absolute reference >> >> + 24 byte RELA entry vs 4 byte relative reference) >> > [..] >> > >> > a side note, >> > checkpatch complaints quite a lot. >> > > [..] >> I will clean up the meaningful ones in v2, but please don't expect >> this series to be checkpatch clean: it simply doesn't deal with inline >> asm very well, and some of this code predates checkpatch by a decade, >> and I'd rather not mix up rather tricky functional changes with >> checkpatch cleanup duty. > > sure. thanks. > > I'm running two x86 boxes with the patch set applied, for > several days, with no issues being observed. it does save > some memory (well, several pages in my case) even on "tiny" > kernels configs. > That is good to hear. Thanks.