On Fri, 21 Mar 2025 10:36:02 +0800
Yuquan Wang <wangyuquan1236@xxxxxxxxxxxxxx> wrote:
> The absence of SRAT would cause the fake_pxm to be -1 and increment
> to 0, then send to acpi_parse_cfmws(). If there exists CXL memory
> ranges that are defined in the CFMWS and not already defined in the
> SRAT, the new node (node0) for the CXL memory would be invalid, as
> node0 is already in "used", and all CXL memory might be online on
> node0.
>
> This utilizes node_set(0, nodes_found_map) to set pxm&node map. With
> this setting, acpi_map_pxm_to_node() could return the expected node
> value even if no SRAT.
>
> If SRAT is valid, the numa_memblks_init() would then utilize
> numa_move_tail_memblk() to move the numa_memblk from numa_meminfo to
> numa_reserved_meminfo in CFMWs fake node situation.
I would call out that numa_move_tail_memblk() is called in
numa_cleanup_meminfo() which is indeed called by num_memblks_init()
>
> If SRAT is missing or bad, the numa_memblks_init() would fail since
> init_func() would fail. And it causes that no numa_memblk in
> numa_reserved_meminfo list and the following dax_cxl driver could
> find the expected fake node.
>
> Use numa_add_reserved_memblk() to replace numa_add_memblk(), since
> the cxl numa_memblk added by numa_add_memblk() would finally be moved
> to numa_reserved_meminfo, and numa_add_reserved_memblk() here could
> add cxl numa_memblk into reserved list directly. Hence, no matter
> SRAT is good or not, cxl numa_memblk could be allocated to reserved
> list.
>
> Signed-off-by: Yuquan Wang <wangyuquan1236@xxxxxxxxxxxxxx>
This definitely wants input from Mike Rapoport.
Looks fine to me, but there may be some subtle corners I'm missing.
> ---
> drivers/acpi/numa/srat.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index 00ac0d7bb8c9..50bfecfb9c16 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -458,11 +458,12 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
> return -EINVAL;
> }
>
> - if (numa_add_memblk(node, start, end) < 0) {
> + if (numa_add_reserved_memblk(node, start, end) < 0) {
> /* CXL driver must handle the NUMA_NO_NODE case */
> pr_warn("ACPI NUMA: Failed to add memblk for CFMWS node %d [mem %#llx-%#llx]\n",
> node, start, end);
> }
> +
Unrelated change. Always give patches a final look through to spot
things like this. Trivial, but they all add noise to what we are focusing on.
> node_set(node, numa_nodes_parsed);
>
> /* Set the next available fake_pxm value */
> @@ -646,8 +647,12 @@ int __init acpi_numa_init(void)
> if (node_to_pxm_map[i] > fake_pxm)
> fake_pxm = node_to_pxm_map[i];
> }
> - last_real_pxm = fake_pxm;
> - fake_pxm++;
> +
> + /* Make sure CFMWs fake node >= 1 */
> + fake_pxm = max(fake_pxm, 0);
> + last_real_pxm = fake_pxm++;
> + node_set(0, nodes_found_map);
> +
> acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
> &fake_pxm);
>
