diff mbox series

[v3,09/16] arch_topology: Drop LLC identifier stash from the CPU topology

Message ID 20220525081416.3306043-10-sudeep.holla@arm.com (mailing list archive)
State New, archived
Headers show
Series arch_topology: Updates to add socket support and fix cluster ids | expand

Commit Message

Sudeep Holla May 25, 2022, 8:14 a.m. UTC
Since the cacheinfo LLC information is used directly in arch_topology,
there is no need to parse and store the LLC ID information only for
ACPI systems in the CPU topology.

Remove the redundant LLC ID from the generic CPU arch_topology information.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/base/arch_topology.c  | 1 -
 include/linux/arch_topology.h | 1 -
 2 files changed, 2 deletions(-)

Comments

Gavin Shan June 1, 2022, 3:35 a.m. UTC | #1
Hi Sudeep,

On 5/25/22 4:14 PM, Sudeep Holla wrote:
> Since the cacheinfo LLC information is used directly in arch_topology,
> there is no need to parse and store the LLC ID information only for
> ACPI systems in the CPU topology.
> 
> Remove the redundant LLC ID from the generic CPU arch_topology information.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/base/arch_topology.c  | 1 -
>   include/linux/arch_topology.h | 1 -
>   2 files changed, 2 deletions(-)
> 

How about merge the changes to PATCH[08/16]? I don't see why we need put
the changes into separate patches.

> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 4c486e4e6f2f..76c702c217c5 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -747,7 +747,6 @@ void __init reset_cpu_topology(void)
>   		cpu_topo->core_id = -1;
>   		cpu_topo->cluster_id = -1;
>   		cpu_topo->package_id = -1;
> -		cpu_topo->llc_id = -1;
>   
>   		clear_cpu_topology(cpu);
>   	}
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 58cbe18d825c..a07b510e7dc5 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -68,7 +68,6 @@ struct cpu_topology {
>   	int core_id;
>   	int cluster_id;
>   	int package_id;
> -	int llc_id;
>   	cpumask_t thread_sibling;
>   	cpumask_t core_sibling;
>   	cpumask_t cluster_sibling;
> 

Thanks,
Gavin
Sudeep Holla June 1, 2022, 12:06 p.m. UTC | #2
On Wed, Jun 01, 2022 at 11:35:20AM +0800, Gavin Shan wrote:
> Hi Sudeep,
> 
> On 5/25/22 4:14 PM, Sudeep Holla wrote:
> > Since the cacheinfo LLC information is used directly in arch_topology,
> > there is no need to parse and store the LLC ID information only for
> > ACPI systems in the CPU topology.
> > 
> > Remove the redundant LLC ID from the generic CPU arch_topology information.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >   drivers/base/arch_topology.c  | 1 -
> >   include/linux/arch_topology.h | 1 -
> >   2 files changed, 2 deletions(-)
> > 
> 
> How about merge the changes to PATCH[08/16]? I don't see why we need put
> the changes into separate patches.
>

It took a while to remember as I was with the same opinion as yours but
decided to split them for one reason: to keep arch specific change in a
separate patch(if that becomes a need due to some conflict or some other
non-technical reason)
Gavin Shan June 2, 2022, 6:42 a.m. UTC | #3
On 5/25/22 4:14 PM, Sudeep Holla wrote:
> Since the cacheinfo LLC information is used directly in arch_topology,
> there is no need to parse and store the LLC ID information only for
> ACPI systems in the CPU topology.
> 
> Remove the redundant LLC ID from the generic CPU arch_topology information.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/base/arch_topology.c  | 1 -
>   include/linux/arch_topology.h | 1 -
>   2 files changed, 2 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 4c486e4e6f2f..76c702c217c5 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -747,7 +747,6 @@ void __init reset_cpu_topology(void)
>   		cpu_topo->core_id = -1;
>   		cpu_topo->cluster_id = -1;
>   		cpu_topo->package_id = -1;
> -		cpu_topo->llc_id = -1;
>   
>   		clear_cpu_topology(cpu);
>   	}
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 58cbe18d825c..a07b510e7dc5 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -68,7 +68,6 @@ struct cpu_topology {
>   	int core_id;
>   	int cluster_id;
>   	int package_id;
> -	int llc_id;
>   	cpumask_t thread_sibling;
>   	cpumask_t core_sibling;
>   	cpumask_t cluster_sibling;
>
Gavin Shan June 2, 2022, 6:44 a.m. UTC | #4
Hi Sudeep,

On 6/1/22 8:06 PM, Sudeep Holla wrote:
> On Wed, Jun 01, 2022 at 11:35:20AM +0800, Gavin Shan wrote:
>> On 5/25/22 4:14 PM, Sudeep Holla wrote:
>>> Since the cacheinfo LLC information is used directly in arch_topology,
>>> there is no need to parse and store the LLC ID information only for
>>> ACPI systems in the CPU topology.
>>>
>>> Remove the redundant LLC ID from the generic CPU arch_topology information.
>>>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>    drivers/base/arch_topology.c  | 1 -
>>>    include/linux/arch_topology.h | 1 -
>>>    2 files changed, 2 deletions(-)
>>>
>>
>> How about merge the changes to PATCH[08/16]? I don't see why we need put
>> the changes into separate patches.
>>
> 
> It took a while to remember as I was with the same opinion as yours but
> decided to split them for one reason: to keep arch specific change in a
> separate patch(if that becomes a need due to some conflict or some other
> non-technical reason)
> 

Ok. Thanks for the explanation, which sounds reasonable to me.

Thanks,
Gavin
diff mbox series

Patch

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 4c486e4e6f2f..76c702c217c5 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -747,7 +747,6 @@  void __init reset_cpu_topology(void)
 		cpu_topo->core_id = -1;
 		cpu_topo->cluster_id = -1;
 		cpu_topo->package_id = -1;
-		cpu_topo->llc_id = -1;
 
 		clear_cpu_topology(cpu);
 	}
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 58cbe18d825c..a07b510e7dc5 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -68,7 +68,6 @@  struct cpu_topology {
 	int core_id;
 	int cluster_id;
 	int package_id;
-	int llc_id;
 	cpumask_t thread_sibling;
 	cpumask_t core_sibling;
 	cpumask_t cluster_sibling;