Re: [PATCH gnumach] cpu_number: Look up cpu kernel_id via lookup table

[Samuel Thibault]

Applied, thanks!

Damien Zammit, le sam. 05 août 2023 07:49:54 +0000, a ecrit:
> This speeds up smp slightly by reducing the cpu_number()
> complexity to have no branching, just a look up table.
> It also addresses the problem that CPU_NUMBER was only using
> raw apic_id as an approximation of the kernel_id.
> Other improvements were to remove unnecessary checks now that
> the lookup table always resolves to a valid value.
> ---
>  i386/i386/apic.c              | 33 ++++++++++++++++++++++-----------
>  i386/i386/apic.h              |  2 ++
>  i386/i386/cpu_number.c        | 19 +------------------
>  i386/i386/cpu_number.h        |  1 +
>  i386/i386at/acpi_parse_apic.c |  2 ++
>  5 files changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> index c399074d..2bb8e3f1 100644
> --- a/i386/i386/apic.c
> +++ b/i386/i386/apic.c
> @@ -35,6 +35,11 @@
>  static ApicLocalUnit dummy_lapic = {0};
>  volatile ApicLocalUnit* lapic = &dummy_lapic;
>  
> +/* This lookup table of [apic_id] -> kernel_id is initially populated with 
> zeroes
> + * so every lookup results in master processor until real kernel ids are 
> populated.
> + */
> +int cpu_id_lut[UINT8_MAX + 1] = {0};
> +
>  ApicInfo apic_data;
>  
>  /*
> @@ -137,14 +142,7 @@ apic_get_cpu_apic_id(int kernel_id)
>  int
>  apic_get_cpu_kernel_id(uint16_t apic_id)
>  {
> -    int i;
> -
> -    for (i = 0; i < apic_data.ncpus; i++) {
> -        if (apic_data.cpu_lapic_list[i] == apic_id)
> -            return i;
> -    }
> -
> -    return -1;
> +    return cpu_id_lut[apic_id];
>  }
>  
>  /* apic_get_lapic: returns a reference to the common memory address for 
> Local APIC. */
> @@ -187,9 +185,6 @@ apic_get_num_ioapics(void)
>  int
>  apic_get_current_cpu(void)
>  {
> -    if(lapic == NULL)
> -        return -1;
> -
>      return (lapic->apic_id.r >> 24) & 0xff;
>  }
>  
> @@ -222,6 +217,22 @@ int apic_refit_cpulist(void)
>      return 0;
>  }
>  
> +/*
> + * apic_generate_cpu_id_lut: Generate lookup table of cpu kernel ids from 
> apic ids
> + */
> +void apic_generate_cpu_id_lut(void)
> +{
> +    int i, apic_id;
> +
> +    for (i = 0; i < apic_data.ncpus; i++) {
> +        apic_id = apic_get_cpu_apic_id(i);
> +        if (apic_id >= 0)
> +            cpu_id_lut[apic_id] = i;
> +        else
> +            printf("apic_get_cpu_apic_id(%d) failed...\n", i);
> +    }
> +}
> +
>  /*
>   * apic_print_info: shows the list of Local APIC and IOAPIC.
>   * Shows each CPU and IOAPIC, with Its Kernel ID and APIC ID.
> diff --git a/i386/i386/apic.h b/i386/i386/apic.h
> index 73e32dfe..e870dcf8 100644
> --- a/i386/i386/apic.h
> +++ b/i386/i386/apic.h
> @@ -238,6 +238,7 @@ uint8_t apic_get_num_ioapics(void);
>  int apic_get_current_cpu(void);
>  void apic_print_info(void);
>  int apic_refit_cpulist(void);
> +void apic_generate_cpu_id_lut(void);
>  void picdisable(void);
>  void lapic_eoi(void);
>  void ioapic_irq_eoi(int pin);
> @@ -250,6 +251,7 @@ void ioapic_configure(void);
>  extern int timer_pin;
>  extern void intnull(int unit);
>  extern volatile ApicLocalUnit* lapic;
> +extern int cpu_id_lut[];
>  
>  #endif
>  
> diff --git a/i386/i386/cpu_number.c b/i386/i386/cpu_number.c
> index 1d2fab3a..ef19e11f 100644
> --- a/i386/i386/cpu_number.c
> +++ b/i386/i386/cpu_number.c
> @@ -25,23 +25,6 @@
>  #if NCPUS > 1
>  int cpu_number(void)
>  {
> -     int kernel_id, apic_id;
> -
> -     if (bspdone == 0)
> -             return 0;
> -
> -     apic_id = apic_get_current_cpu();
> -     if (apic_id < 0) {
> -             printf("apic_get_current_cpu() failed, assuming BSP\n");
> -             apic_id = 0;
> -     }
> -
> -     kernel_id = apic_get_cpu_kernel_id(apic_id);
> -     if (kernel_id < 0) {
> -             printf("apic_get_cpu_kernel_id() failed, assuming BSP\n");
> -             kernel_id = 0;
> -     }
> -
> -     return kernel_id;
> +     return cpu_id_lut[apic_get_current_cpu()];
>  }
>  #endif
> diff --git a/i386/i386/cpu_number.h b/i386/i386/cpu_number.h
> index a5658471..c00896e8 100644
> --- a/i386/i386/cpu_number.h
> +++ b/i386/i386/cpu_number.h
> @@ -43,6 +43,7 @@
>       movl    %cs:lapic, reg          ;\
>       movl    %cs:APIC_ID(reg), reg   ;\
>       shrl    $24, reg                ;\
> +     movl    %cs:CX(cpu_id_lut, reg), reg    ;\
>  
>  #ifndef __ASSEMBLER__
>  #include "kern/cpu_number.h"
> diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
> index 27e3410d..9cd861ed 100644
> --- a/i386/i386at/acpi_parse_apic.c
> +++ b/i386/i386at/acpi_parse_apic.c
> @@ -483,6 +483,8 @@ acpi_apic_setup(struct acpi_apic *apic)
>              return ACPI_FIT_FAILURE;
>      }
>  
> +    apic_generate_cpu_id_lut();
> +
>      return ACPI_SUCCESS;
>  }
>  
> -- 
> 2.40.1
> 
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.