On Wed, Jul 3, 2019 at 5:11 PM Jeremy Linton <jeremy.linton@xxxxxxx> wrote:
>
> Hi,
>
> Thanks for taking a look at this.
>
> On 7/3/19 4:24 AM, Rafael J. Wysocki wrote:
> > On Fri, Jun 28, 2019 at 8:15 PM Jeremy Linton <jeremy.linton@xxxxxxx> wrote:
> >>
> >> ACPI 6.3 adds a flag to the CPU node to indicate whether
> >> the given PE is a thread. Add a function to return that
> >> information for a given linux logical CPU.
> >>
> >> Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
> >> ---
> >> drivers/acpi/pptt.c | 62 +++++++++++++++++++++++++++++++++++++++++++-
> >> include/linux/acpi.h | 5 ++++
> >> 2 files changed, 66 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> >> index b72e6afaa8fb..bb6196422fad 100644
> >> --- a/drivers/acpi/pptt.c
> >> +++ b/drivers/acpi/pptt.c
> >> @@ -517,6 +517,52 @@ static int find_acpi_cpu_topology_tag(unsigned int cpu, int level, int flag)
> >> return retval;
> >> }
> >>
> >> +/**
> >> + * check_acpi_cpu_flag() - Determine if CPU node has a flag set
> >> + * @cpu: Kernel logical CPU number
> >> + * @rev: The PPTT revision defining the flag
> >> + * @flag: The flag itself
> >> + *
> >> + * Check the node representing a CPU for a given flag.
> >> + *
> >> + * Return: -ENOENT if the PPTT doesn't exist, the CPU cannot be found or
> >> + * the table revision isn't new enough.
> >> + * 1, any passed flag set
> >> + * 0, flag unset
> >> + */
> >> +static int check_acpi_cpu_flag(unsigned int cpu, int rev, u32 flag)
> >
> > Why not bool?
>
> At least for the thread flag we need the three states so that we can
> fall back to the CPU's description of itself on machines without ACPI
> 6.3 tables.
>
> The ThunderX2 is threaded and without a firmware update a change like
> this will break it.
Fair enough.
> >
> >> +{
> >> + struct acpi_table_header *table;
> >> + acpi_status status;
> >> + u32 acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> >> + struct acpi_pptt_processor *cpu_node = NULL;
> >> + int ret = -ENOENT;
> >> + static int saved_pptt_rev = -1;
> >> +
> >> + /* Cache the PPTT revision to avoid repeat table get/put on failure */
> >
> > This is a rather questionable optimization.
> >
> > Does the extra table get/put really matter?
>
> AFAIK, Probably not.
Then why to optimize it?
