| 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
| ||
|
Date: Thu, 8 Feb 2018 10:47:10 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Dominik Brodowski <linux@...inikbrodowski.net>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
the arch/x86 maintainers <x86@...nel.org>,
Dan Williams <dan.j.williams@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Andi Kleen <ak@...ux.intel.com>,
Andrew Lutomirski <luto@...nel.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
Borislav Petkov <bp@...en8.de>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [RFC v2 PATCH 6/7] x86/entry: get rid of
ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS
* Dominik Brodowski <linux@...inikbrodowski.net> wrote:
> On Wed, Feb 07, 2018 at 12:44:41PM -0800, Linus Torvalds wrote:
> > On Wed, Feb 7, 2018 at 12:15 PM, Dominik Brodowski
> > <linux@...inikbrodowski.net> wrote:
> > >
> > > Note: The testb $3, CS(%rsp) instruction in idtentry() does not need
> > > modification. Previously %rsp was manually decreased by 15*8; with
> > > this patch, %rsp is decreased by 15 pushq instructions. Moreover,
> > > error_entry did and does the exact same test (with offset=8) after
> > > the registers have been moved/pushed and cleared.
> >
> > So this has the problem that now those save/clear instructions will
> > all be done in that idtentry macro.
> >
> > So now that code will be duplicated for all the users of that macro.
> >
> > The old code did the saving in the common error_entry and
> > paranoid_entry routines, in order to be able to share all the code,
> > and making the duplicated stub functions generated by the idtentry
> > macro smaller.
> >
> > Now, admittedly the new push sequence is much smaller than the old
> > movq sequence, so the duplication doesn't hurt as much, but it's still
> > likely quite noticeable.
> >
> > So this removes lines of asm code, but it adds a lot of instructions
> > to the end result thanks to the macro, I think.
>
> Indeed, that is the case (see below). However, if we want to switch to
> PUSH instructions and do this in a routine which is call'ed and which
> ret'urns, %rsp needs to be moved around even more often than the old
> ALLOC_PT_GPREGS_ON_STACK macro did (which you wanted to get rid of,
> IIUYC). Or do I miss something?
>
> text data bss dec hex filename
> 19500 0 0 19500 4c2c arch/x86/entry/entry_64.o-orig
> 19510 0 0 19510 4c36 arch/x86/entry/entry_64.o-3_of_7
> 21105 0 0 21105 5271 arch/x86/entry/entry_64.o-5_of_7
> 24307 0 0 24307 5ef3 arch/x86/entry/entry_64.o-7_of_7
So while this shows a +~5K increase in text size, these numbers also _very_
significantly over-represent the extra footprint. The assumptions that resulted in
us compressing the IRQ entry code have changed very significantly with the new x86
IRQ allocation code we introduced in the last year:
- IRQ vectors are usually populated in tightly clustered groups.
With our new vector allocator code the typical per CPU allocation percentage on
x86 systems is ~3 device vectors and ~10 fixed vectors out of ~220 vectors -
i.e. a very low ~6% utilization (!). This means that the _real_ text footprint
increase is probably closer to 0.1K of text...
This is a very typical picture on an average desktop system:
/sys/kernel/debug/irq/domains/VECTORS:
Online bitmaps: 40
Global available: 8007
Global reserved: 24
Total allocated: 73
System: 42: 0-19,32,50,128,237-255
| CPU | avl | man | act | vectors
0 199 0 3 33-34,48
1 199 0 3 33-35
2 199 0 3 33-35
3 199 0 3 33-35
4 199 0 3 33-35
5 199 0 3 33-35
6 199 0 3 33-35
7 199 0 3 33-35
8 199 0 3 33-35
9 199 0 3 33-35
10 201 0 1 33
11 201 0 1 33
12 201 0 1 33
13 201 0 1 33
14 201 0 1 33
15 201 0 1 33
16 201 0 1 33
17 201 0 1 33
18 201 0 1 33
19 201 0 1 33
20 199 0 3 33-35
21 199 0 3 33-35
22 199 0 3 33-35
23 199 0 3 33-35
24 199 0 3 33-35
25 199 0 3 33-35
26 199 0 3 33-35
27 199 0 3 33-35
28 199 0 3 33-35
29 199 0 3 33-35
30 201 0 1 33
31 201 0 1 33
32 201 0 1 33
33 202 0 0
34 202 0 0
35 202 0 0
36 202 0 0
37 202 0 0
38 202 0 0
39 202 0 0
I.e. the average number of (device) IRQ vectors per CPU is around 2, with the
max being 3.
The days where we allocated a lot of vectors on every CPU and the compression
of the IRQ entry code text mattered are over.
- Another issue is that only a small minority of vectors is frequent enough to
actually matter to cache utilization in practice: 3-4 key IPIs and 1-2 device
IRQs at most - and those vectors tend to be tightly clustered as well into about
two groups, and are probably already on 2-3 cache lines in practice.
For the common case of 'cache cold' IRQs it's the depth of the call chain and
the fragmentation of the resulting I$ that should be the main performance limit
- not the overall size of it.
- The CPU side cost of IRQ delivery is still very expensive even in the best, most
cached case, as in 'over a thousand cycles'. So much stuff is done that maybe
contemporary x86 IRQ entry microcode already prefetches the IDT entry and its
expected call target address.
So for those reasons I'm really tempted by the all around simplification offered
by this series:
2 files changed, 62 insertions(+), 160 deletions(-)
Basically in this specific case I'd like to turn the argument around,
use this simpler design and put the burden of proof on the patches that
want to _complicate it_ beyond this simple, straightforward model: show us
the numbers that it truly makes a difference: it's not that hard to
spray a CPU with IRQs by using IPIs, and the cache miss rate and the
overall perf counter stats should tell a clear story about whether it
makes sense to cache-compress (and complicate) the IRQ entry sequences.
Thanks,
Ingo
Powered by blists - more mailing lists