lkml.org 
[lkml]   [2019]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 16/18] x86/apic: Convert 32bit to IPI shorthand static key
On Wed, 3 Jul 2019, Nadav Amit wrote:
> > On Jul 3, 2019, at 1:34 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Wed, 3 Jul 2019, Nadav Amit wrote:
> >>> On Jul 3, 2019, at 3:54 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>> void default_send_IPI_all(int vector)
> >>> {
> >>> - if (apic_ipi_shorthand_off || vector == NMI_VECTOR) {
> >>> + if (static_branch_likely(&apic_use_ipi_shorthand)) {
> >>> apic->send_IPI_mask(cpu_online_mask, vector);
> >>> } else {
> >>> __default_send_IPI_shortcut(APIC_DEST_ALLINC, vector);
> >>
> >> It may be better to check the static-key in native_send_call_func_ipi() (and
> >> other callers if there are any), and remove all the other checks in
> >> default_send_IPI_all(), x2apic_send_IPI_mask_allbutself(), etc.
> >
> > That makes sense. Should have thought about that myself, but hunting that
> > APIC emulation issue was affecting my brain obviously :)
>
> Well, if you used VMware and not KVM... ;-)

Then I would have hunted some other bug probably :)

> >> void native_send_call_func_ipi(const struct cpumask *mask)
> >> {
> >> - cpumask_var_t allbutself;
> >> -
> >> - if (!alloc_cpumask_var(&allbutself, GFP_ATOMIC)) {
> >> - apic->send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
> >> - return;
> >> + int cpu, this_cpu = smp_processor_id();
> >> + bool allbutself = true;
> >> + bool self = false;
> >> +
> >> + for_each_cpu_and_not(cpu, cpu_online_mask, mask) {
> >> +
> >> + if (cpu != this_cpu) {
> >> + allbutself = false;
> >> + break;
> >> + }
> >> + self = true;
> >
> > That accumulates to a large iteration in the worst case.
>
> I don’t understand why. There should be at most two iterations - one for
> self and one for another core. So _find_next_bit() will be called at most
> twice. _find_next_bit() has its own loop, but I don’t think overall it is as

Indeed, misread the code and right the bit search should be fast.

> bad as calling alloc_cpumask_var(), cpumask_copy() and cpumask_equal(),
> which also have loops.
>
> I don’t have numbers (and I doubt they are very significant), but the cpumask
> allocation showed when I was profiling my microbenchmark.

Yes, that alloc/free part is completely bogus.

Thanks,

tglx
\
 
 \ /
  Last update: 2019-07-03 23:31    [W:0.055 / U:8.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site