[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZSpnpfn/mSYrgC9C@gmail.com>
Date: Sat, 14 Oct 2023 12:04:21 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Uros Bizjak <ubizjak@...il.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
Linus Torvalds <torvalds@...ux-foundation.org>,
Nadav Amit <namit@...are.com>,
Andy Lutomirski <luto@...nel.org>,
Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
"H . Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH tip] x86/percpu: Rewrite arch_raw_cpu_ptr()
* Uros Bizjak <ubizjak@...il.com> wrote:
> Implement arch_raw_cpu_ptr() as a load from this_cpu_off and then
> add the ptr value to the base. This way, the compiler can propagate
> addend to the following instruction and simplify address calculation.
>
> E.g.: address calcuation in amd_pmu_enable_virt() improves from:
>
> 48 c7 c0 00 00 00 00 mov $0x0,%rax
> 87b7: R_X86_64_32S cpu_hw_events
>
> 65 48 03 05 00 00 00 add %gs:0x0(%rip),%rax
> 00
> 87bf: R_X86_64_PC32 this_cpu_off-0x4
>
> 48 c7 80 28 13 00 00 movq $0x0,0x1328(%rax)
> 00 00 00 00
>
> to:
>
> 65 48 8b 05 00 00 00 mov %gs:0x0(%rip),%rax
> 00
> 8798: R_X86_64_PC32 this_cpu_off-0x4
> 48 c7 80 00 00 00 00 movq $0x0,0x0(%rax)
> 00 00 00 00
> 87a6: R_X86_64_32S cpu_hw_events+0x1328
>
> The compiler can also eliminate redundant loads from this_cpu_off,
> reducing the number of percpu offset reads (either from this_cpu_off
> or with rdgsbase) from 1663 to 1571.
>
> Additionaly, the patch introduces 'rdgsbase' alternative for CPUs with
> X86_FEATURE_FSGSBASE. The rdgsbase instruction *probably* will end up
> only decoding in the first decoder etc. But we're talking single-cycle
> kind of effects, and the rdgsbase case should be much better from
> a cache perspective and might use fewer memory pipeline resources to
> offset the fact that it uses an unusual front end decoder resource...
So the 'additionally' wording in the changelog should have been a big hint
already that the introduction of RDGSBASE usage needs to be a separate
patch. ;-)
Thanks,
Ingo
Powered by blists - more mailing lists