[PATCH] ACPI: NUMA: Move get_numa_distances_cnt() helper to needed location

WangYuli posted 1 patch 10 months ago
arch/loongarch/kernel/acpi.c |  5 -----
drivers/acpi/numa/srat.c     | 13 +++++++++----
2 files changed, 9 insertions(+), 9 deletions(-)
[PATCH] ACPI: NUMA: Move get_numa_distances_cnt() helper to needed location
Posted by WangYuli 10 months ago
In LoongArch, get_numa_distances_cnt() was not in use, resulting in
a compiler warning.

Serendipitously, drivers/acpi/numa/srat.c appears to be a more
relevant location for this helper function, hence its relocation.

This commit not only resolves these immediate concerns but also sets
the groundwork for potential future integration of ACPI related logic
from other architectures into this driver module.

Separately, the locality_count member in struct acpi_table_slit is
typed as u64. Adapt the function type to eliminate potential code
risks.

Fix follow errors with clang-18 when W=1e:

arch/loongarch/kernel/acpi.c:259:28: error: unused function 'get_numa_distances_cnt' [-Werror,-Wunused-function]
  259 | static inline unsigned int get_numa_distances_cnt(struct acpi_table_slit *slit)
      |                            ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 arch/loongarch/kernel/acpi.c |  5 -----
 drivers/acpi/numa/srat.c     | 13 +++++++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
index ee471a80763e..90cc9250f121 100644
--- a/arch/loongarch/kernel/acpi.c
+++ b/arch/loongarch/kernel/acpi.c
@@ -256,11 +256,6 @@ static __init int setup_node(int pxm)
  */
 unsigned int numa_distance_cnt;
 
-static inline unsigned int get_numa_distances_cnt(struct acpi_table_slit *slit)
-{
-	return slit->locality_count;
-}
-
 void __init numa_set_distance(int from, int to, int distance)
 {
 	if ((u8)distance != distance || (from == to && distance != LOCAL_DISTANCE)) {
diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index 00ac0d7bb8c9..36053ae3dad6 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -283,6 +283,11 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 	}
 }
 
+static inline u64 get_numa_distances_cnt(struct acpi_table_slit *slit)
+{
+	return slit->locality_count;
+}
+
 /*
  * A lot of BIOS fill in 10 (= no distance) everywhere. This messes
  * up the NUMA heuristics which wants the local node to have a smaller
@@ -292,7 +297,7 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
 static int __init slit_valid(struct acpi_table_slit *slit)
 {
 	int i, j;
-	int d = slit->locality_count;
+	u64 d = get_numa_distances_cnt(slit);
 	for (i = 0; i < d; i++) {
 		for (j = 0; j < d; j++) {
 			u8 val = slit->entry[d*i + j];
@@ -337,20 +342,20 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < slit->locality_count; i++) {
+	for (i = 0; i < get_numa_distances_cnt(slit); i++) {
 		const int from_node = pxm_to_node(i);
 
 		if (from_node == NUMA_NO_NODE)
 			continue;
 
-		for (j = 0; j < slit->locality_count; j++) {
+		for (j = 0; j < get_numa_distances_cnt(slit); j++) {
 			const int to_node = pxm_to_node(j);
 
 			if (to_node == NUMA_NO_NODE)
 				continue;
 
 			numa_set_distance(from_node, to_node,
-				slit->entry[slit->locality_count * i + j]);
+				slit->entry[get_numa_distances_cnt(slit) * i + j]);
 		}
 	}
 
-- 
2.47.2
Re: [PATCH] ACPI: NUMA: Move get_numa_distances_cnt() helper to needed location
Posted by Mike Rapoport 10 months ago
Hi,

On Thu, Feb 20, 2025 at 12:20:37PM +0800, WangYuli wrote:
> In LoongArch, get_numa_distances_cnt() was not in use, resulting in
> a compiler warning.
> 
> Serendipitously, drivers/acpi/numa/srat.c appears to be a more
> relevant location for this helper function, hence its relocation.

There's no need for relocation, just drop the unused function.
 
> This commit not only resolves these immediate concerns but also sets
> the groundwork for potential future integration of ACPI related logic
> from other architectures into this driver module.
> 
> Separately, the locality_count member in struct acpi_table_slit is
> typed as u64. Adapt the function type to eliminate potential code
> risks.
> 
> Fix follow errors with clang-18 when W=1e:
> 
> arch/loongarch/kernel/acpi.c:259:28: error: unused function 'get_numa_distances_cnt' [-Werror,-Wunused-function]
>   259 | static inline unsigned int get_numa_distances_cnt(struct acpi_table_slit *slit)
>       |                            ^~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> 
> Signed-off-by: WangYuli <wangyuli@uniontech.com>
> ---
>  arch/loongarch/kernel/acpi.c |  5 -----
>  drivers/acpi/numa/srat.c     | 13 +++++++++----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> index ee471a80763e..90cc9250f121 100644
> --- a/arch/loongarch/kernel/acpi.c
> +++ b/arch/loongarch/kernel/acpi.c
> @@ -256,11 +256,6 @@ static __init int setup_node(int pxm)
>   */
>  unsigned int numa_distance_cnt;
>  
> -static inline unsigned int get_numa_distances_cnt(struct acpi_table_slit *slit)
> -{
> -	return slit->locality_count;
> -}
> -
>  void __init numa_set_distance(int from, int to, int distance)
>  {
>  	if ((u8)distance != distance || (from == to && distance != LOCAL_DISTANCE)) {
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index 00ac0d7bb8c9..36053ae3dad6 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -283,6 +283,11 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
>  	}
>  }
>  
> +static inline u64 get_numa_distances_cnt(struct acpi_table_slit *slit)
> +{
> +	return slit->locality_count;
> +}
> +
>  /*
>   * A lot of BIOS fill in 10 (= no distance) everywhere. This messes
>   * up the NUMA heuristics which wants the local node to have a smaller
> @@ -292,7 +297,7 @@ acpi_table_print_srat_entry(struct acpi_subtable_header *header)
>  static int __init slit_valid(struct acpi_table_slit *slit)
>  {
>  	int i, j;
> -	int d = slit->locality_count;
> +	u64 d = get_numa_distances_cnt(slit);
>  	for (i = 0; i < d; i++) {
>  		for (j = 0; j < d; j++) {
>  			u8 val = slit->entry[d*i + j];
> @@ -337,20 +342,20 @@ static int __init acpi_parse_slit(struct acpi_table_header *table)
>  		return -EINVAL;
>  	}
>  
> -	for (i = 0; i < slit->locality_count; i++) {
> +	for (i = 0; i < get_numa_distances_cnt(slit); i++) {
>  		const int from_node = pxm_to_node(i);
>  
>  		if (from_node == NUMA_NO_NODE)
>  			continue;
>  
> -		for (j = 0; j < slit->locality_count; j++) {
> +		for (j = 0; j < get_numa_distances_cnt(slit); j++) {
>  			const int to_node = pxm_to_node(j);
>  
>  			if (to_node == NUMA_NO_NODE)
>  				continue;
>  
>  			numa_set_distance(from_node, to_node,
> -				slit->entry[slit->locality_count * i + j]);
> +				slit->entry[get_numa_distances_cnt(slit) * i + j]);
>  		}
>  	}
>  
> -- 
> 2.47.2
> 

-- 
Sincerely yours,
Mike.
Re: [PATCH] ACPI: NUMA: Move get_numa_distances_cnt() helper to needed location
Posted by WangYuli 10 months ago
Hi Mike,

On 2025/2/20 14:10, Mike Rapoport wrote:
> There's no need for relocation, just drop the unused function.

Okay.

But  please take a look at line 295 of the original srat.c. Should the 
type of variable 'd' there be changed to u64, as mentioned in the commit 
message?

If yes, I can quickly put up another commit just to tweak this one place.

Thanks,

-- 
WangYuli
Re: [PATCH] ACPI: NUMA: Move get_numa_distances_cnt() helper to needed location
Posted by Mike Rapoport 9 months, 4 weeks ago
Hi,

On Thu, Feb 20, 2025 at 02:25:33PM +0800, WangYuli wrote:
> Hi Mike,
> 
> On 2025/2/20 14:10, Mike Rapoport wrote:
> > There's no need for relocation, just drop the unused function.
> 
> Okay.
> 
> But  please take a look at line 295 of the original srat.c. Should the type
> of variable 'd' there be changed to u64, as mentioned in the commit message?

int is enough for more than 2 million nodes, I don't see a problem with it
 
> If yes, I can quickly put up another commit just to tweak this one place.
> 
> Thanks,
> 
> -- 
> WangYuli






-- 
Sincerely yours,
Mike.