linux-kernel - RE: [PATCH v2] cpu-topology: Skip the exist but not possible cpu nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Jan 2020 12:17:41 +0000
From:   "Zengtao (B)" <prime.zeng@...ilicon.com>
To:     Sudeep Holla <sudeep.holla@....com>
CC:     Linuxarm <linuxarm@...wei.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] cpu-topology: Skip the exist but not possible cpu
 nodes

> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@....com]
> Sent: Tuesday, January 14, 2020 6:30 PM
> To: Zengtao (B)
> Cc: Linuxarm; Greg Kroah-Hartman; Rafael J. Wysocki;
> linux-kernel@...r.kernel.org; Sudeep Holla
> Subject: Re: [PATCH v2] cpu-topology: Skip the exist but not possible cpu
> nodes
> 
> On Tue, Jan 14, 2020 at 01:42:25AM +0000, Zengtao (B) wrote:
> > Could you help to explain here?
> > I understand there are two abnormal cases:
> > 1. The cpu node exist in the device tree, but not a possible cpu.
> > This case can be caught by of_cpu_node_to_id's return value.
> 
> Yes if of_cpu_node_to_id returns -ENODEV, it means there's no logical
> CPU associated with this DT node.
> 
> > 2. The cpu node does not exist. This case can be caught by above logic.
> Or
> > do you think of_parse_phandle's return value is enough?
> 
> Again yes, there's nothing extra needed.
> 
> The only change you need is to consider -ENODEV while handling the
> case(1)
> 
Thanks very much for your explanation.
So finally it turns into a very simple patch like this, more cleaner:
+/*
+ * This function returns the logic cpu number of the node.
+ * There are basically three kinds of return values:
+ * (1) logic cpu number which is > 0.
+ * (2) -ENDEV when the node is valid one which can be found in the device tree
+ * but there is no possible cpu nodes to match, when the CONFIG_NR_CPUS is
+ * smaller than cpus node numbers in device tree, this will happen. It's
+ * suggested to just ignore this case.
+ * (3) -1 if the node does not exist in the device tree
+ */
 static int __init get_cpu_for_node(struct device_node *node)
 {
        struct device_node *cpu_node;
@@ -261,7 +271,8 @@ static int __init get_cpu_for_node(struct device_node *node)
        if (cpu >= 0)
                topology_parse_cpu_capacity(cpu_node, cpu);
        else
-               pr_crit("Unable to find CPU node for %pOF\n", cpu_node);
+               pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
+                       cpu_node, cpumask_pr_args(cpu_possible_mask));

        of_node_put(cpu_node);
        return cpu;
@@ -286,9 +297,8 @@ static int __init parse_core(struct device_node *core, int package_id,
                                cpu_topology[cpu].package_id = package_id;
                                cpu_topology[cpu].core_id = core_id;
                                cpu_topology[cpu].thread_id = i;
-                       } else {
-                               pr_err("%pOF: Can't get CPU for thread\n",
-                                      t);
+                       } else if (cpu != -ENODEV) {
+                               pr_err("%pOF: Can't get CPU for thread\n", t);
                                of_node_put(t);
                                return -EINVAL;
                        }
@@ -307,7 +317,7 @@ static int __init parse_core(struct device_node *core, int package_id,

                cpu_topology[cpu].package_id = package_id;
                cpu_topology[cpu].core_id = core_id;
-       } else if (leaf) {
+       } else if (leaf && cpu != -ENODEV) {
                pr_err("%pOF: Can't get CPU for leaf core\n", core);
                return -EINVAL;
        }

Any more suggestions? 

Regards
Zengtao