linux-kernel - Re: [RFC] percpu section full of holes

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150608030932.GB21465@mtj.duckdns.org>
Date:	Mon, 8 Jun 2015 12:09:32 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [RFC] percpu section full of holes

Hello, Eric.

On Sun, Jun 07, 2015 at 09:55:07AM -0700, Eric Dumazet wrote:
> In commit bdf977b37418cdf8a2252504779a7e12a09b7575
> ("x86, percpu: Collect hot percpu variables into one cacheline")
> 
> You wrote that forcing ____cacheline_aligned on
> current_task would put all hot variables together.
>
> However this seems not generally true.

Heh, that's way back.  It's likely something caused by changes in the
toolchain.

...
> compiler/linker do not seem to care about the order in the source file.

Indeed.  It seems to be related to the top level reordering.
-fno-toplevel-reorder seems to behave weirdly on gcc 5.1.1 - the
option reverses the symbols but the order is different from the source
code order regardless of the option.

Source code order.

 kernel_stack
 current_task
 irq_stack_ptr
 irq_count
 __preempt_count
 fpu_owner_task
 orig_ist
 debug_stack_usage
 debug_idt_ctr

w/o -fno-toplevel-reorder

 193: 0000000000000008     8 OBJECT  GLOBAL DEFAULT   27 fpu_owner_task
 151: 0000000000000010     4 OBJECT  GLOBAL DEFAULT   27 debug_stack_usage
 153: 0000000000000014     4 OBJECT  GLOBAL DEFAULT   27 debug_idt_ctr
 172: 0000000000000018     4 OBJECT  GLOBAL DEFAULT   27 __preempt_count
 187: 0000000000000020     8 OBJECT  GLOBAL DEFAULT   27 kernel_stack
 195: 0000000000000028     4 OBJECT  GLOBAL DEFAULT   27 irq_count
 196: 0000000000000030     8 OBJECT  GLOBAL DEFAULT   27 irq_stack_ptr
 164: 0000000000000040    56 OBJECT  GLOBAL DEFAULT   27 orig_ist
 167: 0000000000000080     8 OBJECT  GLOBAL DEFAULT   27 current_task
			   
w/ -fno-toplevel-reorder (mirror image of w/o -fno-toplevel-reorder?????)

  97: 0000000000000000     8 OBJECT  GLOBAL DEFAULT    6 current_task
 106: 0000000000000020    56 OBJECT  GLOBAL DEFAULT    6 orig_ist
 108: 0000000000000058     8 OBJECT  GLOBAL DEFAULT    6 irq_stack_ptr
 110: 0000000000000060     4 OBJECT  GLOBAL DEFAULT    6 irq_count
 111: 0000000000000068     8 OBJECT  GLOBAL DEFAULT    6 kernel_stack
 113: 0000000000000070     4 OBJECT  GLOBAL DEFAULT    6 __preempt_count
 115: 0000000000000074     4 OBJECT  GLOBAL DEFAULT    6 debug_idt_ctr
 116: 0000000000000078     4 OBJECT  GLOBAL DEFAULT    6 debug_stack_usage
 117: 0000000000000080     8 OBJECT  GLOBAL DEFAULT    6 fpu_owner_task

and then the linker goes onto interpose symbols from different object
files.

...
> I wish we had a way to remove the automatic alignment of ELF sections
> based on size of objects/structures.
> Why __alignof__ is not respected I dont know.

It's still following the alignments specified tho.  It's just that all
the symbols get shuffled.

> linker propagates the biggest alignment to various built-in.o builds,
> and we have all these holes.
> 
> objdump -h kernel/built-in.o | grep data..percpu
>  28 .data..percpu 00003ce0  0000000000000000  0000000000000000  0011c940  2**6
> 283 .data..percpu..shared_aligned 00001720  0000000000000000  0000000000000000  00146e80  2**6
> 
> We might add a DEFINE_PER_CPU_SMALL() for objects less than sizeof(long),
> but this would mean a lot of changes in the tree.

ld has --sort-common option which sorts symbols by their alignments
which isn't ideal but could help packing smaller percpu variables.  I
can't find a way to prevent ld from interposing symbols and we seem to
have lost control over offset placement no matter what.

> And we would still have holes when per cpu objects are 40 bytes long,
> because we would not use the 24 bytes after them.

It's not like we didn't have holes before tho.  We just had holes
according to source code order and could control it when we explicitly
wanted to.  Overall, compiler / linker reshuffling them at least has
possibility of packing things better.

For things we want to pack together, I suppose our only option is
packing them into a struct and defining access macros into it.  It's
unfortunately clumsier but expresses the intended effect clearer to
both readers and toolchain.

Thanks.

-- 
tejun
--
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/