Re: [PATCH v3 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 7/3/19 4:57 PM, Rafael J. Wysocki wrote:
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?

There was some discussion in the v2 review thread about all the get/put operations which only existed to return failure for each core in the machine.

https://www.spinics.net/lists/arm-kernel/msg735948.html

I guess I should drop it, until we have some proof that there is a problem.





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux