Hi Igor,
On 10/12/21 8:40 PM, Igor Mammedov wrote:
On Wed, 6 Oct 2021 18:22:08 +0800
Gavin Shan <gs...@redhat.com> wrote:
The following option is used to specify the distance map. It's
possible the option isn't provided by user. In this case, the
distance map isn't populated and exposed to platform. On the
other hand, the empty NUMA node, where no memory resides, is
allowed on ARM64 virt platform. For these empty NUMA nodes,
their corresponding device-tree nodes aren't populated, but
their NUMA IDs should be included in the "/distance-map"
device-tree node, so that kernel can probe them properly if
device-tree is used.
-numa,dist,src=<numa_id>,dst=<numa_id>,val=<distance>
So when user doesn't specify distance map, we need to generate
the default distance map, where the local and remote distances
are 10 and 20 separately. This adds an extra parameter to the
exiting complete_init_numa_distance() to generate the default
distance map for this case.
Signed-off-by: Gavin Shan <gs...@redhat.com>
how about error-ing out if distance map is required but
not provided by user explicitly and asking user to fix
command line?
Reasoning behind this that defaults are hard to maintain
and will require compat hacks and being raod blocks down
the road.
Approach I was taking with generic NUMA code, is deprecating
defaults and replacing them with sanity checks, which bail
out on incorrect configuration and ask user to correct command line.
Hence I dislike approach taken in this patch.
If you really wish to provide default, push it out of
generic code into ARM specific one
(then I won't oppose it that much (I think PPC does
some magic like this))
Also behavior seems to be ARM specific so generic
NUMA code isn't a place for it anyways
Thanks for your comments.
Yep, Lets move the logic into hw/arm/virt in v3 because I think simply
error-ing out will block the existing configuration where the distance
map isn't provided by user. After moving the logic to hw/arm/virt,
this patch is consistent with PATCH[02/02] and the specific platform
is affected only.
---
hw/core/numa.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 510d096a88..fdb3a4aeca 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -594,7 +594,7 @@ static void validate_numa_distance(MachineState *ms)
}
}
-static void complete_init_numa_distance(MachineState *ms)
+static void complete_init_numa_distance(MachineState *ms, bool is_default)
{
int src, dst;
NodeInfo *numa_info = ms->numa_state->nodes;
@@ -609,6 +609,8 @@ static void complete_init_numa_distance(MachineState *ms)
if (numa_info[src].distance[dst] == 0) {
if (src == dst) {
numa_info[src].distance[dst] = NUMA_DISTANCE_MIN;
+ } else if (is_default) {
+ numa_info[src].distance[dst] = NUMA_DISTANCE_DEFAULT;
} else {
numa_info[src].distance[dst] =
numa_info[dst].distance[src];
}
@@ -716,13 +718,20 @@ void numa_complete_configuration(MachineState *ms)
* A->B != distance B->A, then that means the distance table is
* asymmetric. In this case, the distances for both directions
* of all node pairs are required.
+ *
+ * The default node pair distances, which are 10 and 20 for the
+ * local and remote nodes separatly, are provided if user doesn't
+ * specify any node pair distances.
*/
if (ms->numa_state->have_numa_distance) {
/* Validate enough NUMA distance information was provided. */
validate_numa_distance(ms);
/* Validation succeeded, now fill in any missing distances. */
- complete_init_numa_distance(ms);
+ complete_init_numa_distance(ms, false);
+ } else {
+ complete_init_numa_distance(ms, true);
+ ms->numa_state->have_numa_distance = true;
}
}
}
Thanks,
Gavin
