[PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Zhao Yakui]

Subject: ACPI : Update the t-state for every affected cpu when t-state is changed
>From : Zhao Yakui <yakui.zhao@intel.com>

According to ACPI spec, the _TSD object provides T-state control cross
logical processor dependency information to OSPM. So the t-state
coordination should be considered when T-state for one cpu is changed.

According to ACPI spec, three types of coordination are defined.
SW_ALL, SW_ANY and HW_ALL.
SW_ALL: it means that OSPM needs to initiate T-state transition on 
all processors in the domain. It is necessary to call throttling set function
for all affected cpus.
SW_ANY: it means that OSPM may initiate T-state transition on any processor in 
the domain. 
HW_ALL: Apec only says that hardware will perform the coordination and doesn't 
recommend how OSPM coordinate T-state among the affected cpus. So it is treated
as the type of SW_ALL. It means that OSPM needs to initiate t-state transition
on all the processors in the domain.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/acpi/processor_throttling.c |   79 +++++++++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 5 deletions(-)

Index: linux-2.6/drivers/acpi/processor_throttling.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_throttling.c
+++ linux-2.6/drivers/acpi/processor_throttling.c
@@ -68,7 +68,7 @@ static int acpi_processor_update_tsd_coo
 
 	/*
 	 * Now that we have _TSD data from all CPUs, lets setup T-state
-	 * coordination among all CPUs.
+	 * coordination between all CPUs.
 	 */
 	for_each_possible_cpu(i) {
 		pr = processors[i];
@@ -988,6 +988,11 @@ int acpi_processor_set_throttling(struct
 {
 	cpumask_t saved_mask;
 	int ret;
+	unsigned int i;
+	struct acpi_processor *match_pr;
+	struct acpi_processor_throttling *p_throttling;
+	struct throttling_tstate t_state;
+	cpumask_t online_throttling_cpus;
 
 	if (!pr)
 		return -EINVAL;
@@ -998,12 +1003,76 @@ int acpi_processor_set_throttling(struct
 	if ((state < 0) || (state > (pr->throttling.state_count - 1)))
 		return -EINVAL;
 
+	saved_mask = current->cpus_allowed;
+	t_state.target_state = state;
+	p_throttling = &(pr->throttling);
+	cpus_and(online_throttling_cpus, cpu_online_map,
+			p_throttling->shared_cpu_map);
 	/*
-	 * Migrate task to the cpu pointed by pr.
+	 * The throttling notifier will be called for every
+	 * affected cpu in order to get one proper T-state.
+	 * The notifier event is THROTTLING_PRECHANGE.
 	 */
-	saved_mask = current->cpus_allowed;
-	set_cpus_allowed(current, cpumask_of_cpu(pr->id));
-	ret = pr->throttling.acpi_processor_set_throttling(pr, state);
+	for_each_cpu_mask(i, online_throttling_cpus) {
+		t_state.cpu = i;
+		acpi_processor_throttling_notifier(THROTTLING_PRECHANGE,
+							&t_state);
+	}
+	/*
+	 * The function of acpi_processor_set_throttling will be called
+	 * to switch T-state. If the coordination type is SW_ALL or HW_ALL,
+	 * it is necessary to call it for every affected cpu. Otherwise
+	 * it can be called only for the cpu pointed by pr.
+	 */
+	if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) {
+		set_cpus_allowed(current, cpumask_of_cpu(pr->id));
+		ret = p_throttling->acpi_processor_set_throttling(pr,
+						t_state.target_state);
+	} else {
+		/*
+		 * When the T-state coordination is SW_ALL or HW_ALL,
+		 * it is necessary to set T-state for every affected
+		 * cpus.
+		 */
+		for_each_cpu_mask(i, online_throttling_cpus) {
+			match_pr = processors[i];
+			/*
+			 * If the pointer is invalid, we will report the
+			 * error message and continue.
+			 */
+			if (!match_pr) {
+				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+					"Invalid Pointer for CPU %d\n", i));
+				continue;
+			}
+			/*
+			 * If the throttling control is unsupported on CPU i,
+			 * we will report the error message and continue.
+			 */
+			if (!match_pr->flags.throttling) {
+				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+					"Throttling Controll is unsupported "
+					"on CPU %d\n", i));
+				continue;
+			}
+			t_state.cpu = i;
+			set_cpus_allowed(current, cpumask_of_cpu(i));
+			ret = match_pr->throttling.
+				acpi_processor_set_throttling(
+				match_pr, t_state.target_state);
+		}
+	}
+	/*
+	 * After the set_throttling is called, the
+	 * throttling notifier is called for every
+	 * affected cpu to update the T-states.
+	 * The notifier event is THROTTLING_POSTCHANGE
+	 */
+	for_each_cpu_mask(i, online_throttling_cpus) {
+		t_state.cpu = i;
+		acpi_processor_throttling_notifier(THROTTLING_POSTCHANGE,
+							&t_state);
+	}
 	/* restore the previous state */
 	set_cpus_allowed(current, saved_mask);
 	return ret;

Re: [PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Len Brown]

On Monday 28 January 2008 00:55, Zhao Yakui wrote:
> Subject: ACPI : Update the t-state for every affected cpu when t-state is changed
> >From : Zhao Yakui <yakui.zhao@intel.com>
> 
> According to ACPI spec, the _TSD object provides T-state control cross
> logical processor dependency information to OSPM. So the t-state
> coordination should be considered when T-state for one cpu is changed.
> 
> According to ACPI spec, three types of coordination are defined.
> SW_ALL, SW_ANY and HW_ALL.

> SW_ALL: it means that OSPM needs to initiate T-state transition on 
> all processors in the domain. It is necessary to call throttling set function
> for all affected cpus.

> SW_ANY: it means that OSPM may initiate T-state transition on any processor in 
> the domain. 

> HW_ALL: Apec only says that hardware will perform the coordination and doesn't 
> recommend how OSPM coordinate T-state among the affected cpus. So it is treated
> as the type of SW_ALL. It means that OSPM needs to initiate t-state transition
> on all the processors in the domain.

Not really. HW_ALL means that the OS can assume that the CPUs are independent,
and hardware will take care of any coordination needed.

ie. the OS should implement throttling changes on each CPU depending
only on that CPU's needs and not need to consider any effects on other CPUs.

This is not the same as SW_ALL, which tells us that all the CPUS in a domain
are _dependent_, and the OS must enforce that by writing the _same_ value
to each CPU.

say you have a domain with 2 CPUS.
If it is HW_ALL and you throttle one cpu, then you are done.
if it is SW_ALL then if you want to throttle one cpu, then you must
also throttle the other.

-Len

Re: [PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Zhao Yakui]

On Sat, 2008-02-02 at 02:39 -0500, Len Brown wrote:
> On Monday 28 January 2008 00:55, Zhao Yakui wrote:
> > Subject: ACPI : Update the t-state for every affected cpu when t-state is changed
> > >From : Zhao Yakui <yakui.zhao@intel.com>
> > 
> > According to ACPI spec, the _TSD object provides T-state control cross
> > logical processor dependency information to OSPM. So the t-state
> > coordination should be considered when T-state for one cpu is changed.
> > 
> > According to ACPI spec, three types of coordination are defined.
> > SW_ALL, SW_ANY and HW_ALL.
> 
> > SW_ALL: it means that OSPM needs to initiate T-state transition on 
> > all processors in the domain. It is necessary to call throttling set function
> > for all affected cpus.
> 
> > SW_ANY: it means that OSPM may initiate T-state transition on any processor in 
> > the domain. 
> 
> > HW_ALL: Apec only says that hardware will perform the coordination and doesn't 
> > recommend how OSPM coordinate T-state among the affected cpus. So it is treated
> > as the type of SW_ALL. It means that OSPM needs to initiate t-state transition
> > on all the processors in the domain.
> 
> Not really. HW_ALL means that the OS can assume that the CPUs are independent,
> and hardware will take care of any coordination needed. 
 Hardware will be responsible for coordinating the state transition
between multiple processors. But hardware should provide OSPM with a
means to determine the actual state residency so that the correct state
is entered. 
> 
> ie. the OS should implement throttling changes on each CPU depending
> only on that CPU's needs and not need to consider any effects on other CPUs.
> 
> This is not the same as SW_ALL, which tells us that all the CPUS in a domain
> are _dependent_, and the OS must enforce that by writing the _same_ value
> to each CPU. 
> 
> say you have a domain with 2 CPUS.
> If it is HW_ALL and you throttle one cpu, then you are done. 
   In this case , how to update the state of another cpu? Can it be
treated as SW_ANY?  
> if it is SW_ALL then if you want to throttle one cpu, then you must
> also throttle the other. 
   I agree. It is necessary to initiate the state transition for every
affected cpu. 
> 
> -Len

Re: [PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Len Brown]

On Saturday 02 February 2008 05:10, Zhao Yakui wrote:
> On Sat, 2008-02-02 at 02:39 -0500, Len Brown wrote:
> > On Monday 28 January 2008 00:55, Zhao Yakui wrote:
> > > Subject: ACPI : Update the t-state for every affected cpu when t-state is changed
> > > >From : Zhao Yakui <yakui.zhao@intel.com>
> > > 
> > > According to ACPI spec, the _TSD object provides T-state control cross
> > > logical processor dependency information to OSPM. So the t-state
> > > coordination should be considered when T-state for one cpu is changed.
> > > 
> > > According to ACPI spec, three types of coordination are defined.
> > > SW_ALL, SW_ANY and HW_ALL.
> > 
> > > SW_ALL: it means that OSPM needs to initiate T-state transition on 
> > > all processors in the domain. It is necessary to call throttling set function
> > > for all affected cpus.
> > 
> > > SW_ANY: it means that OSPM may initiate T-state transition on any processor in 
> > > the domain. 
> > 
> > > HW_ALL: Apec only says that hardware will perform the coordination and doesn't 
> > > recommend how OSPM coordinate T-state among the affected cpus. So it is treated
> > > as the type of SW_ALL. It means that OSPM needs to initiate t-state transition
> > > on all the processors in the domain.
> > 
> > Not really. HW_ALL means that the OS can assume that the CPUs are independent,
> > and hardware will take care of any coordination needed. 
>  Hardware will be responsible for coordinating the state transition
> between multiple processors. But hardware should provide OSPM with a
> means to determine the actual state residency so that the correct state
> is entered. 

The OS has ACNT/MCNT to find the actual frequency.
We use it already in the P-state code to detect when the hardware does
soemthing funky with P-state, such as HW coordination,
TM1/TM2, or turbo mode.

> > 
> > ie. the OS should implement throttling changes on each CPU depending
> > only on that CPU's needs and not need to consider any effects on other CPUs.
> > 
> > This is not the same as SW_ALL, which tells us that all the CPUS in a domain
> > are _dependent_, and the OS must enforce that by writing the _same_ value
> > to each CPU. 
> > 
> > say you have a domain with 2 CPUS.
> > If it is HW_ALL and you throttle one cpu, then you are done. 
>    In this case , how to update the state of another cpu? Can it be
> treated as SW_ANY?  

That is the whole point of HW_ALL -- you don't have to worry
about the "other" CPU, you worry just about your local decisions
and the HW takes care of any dependencies.

> > if it is SW_ALL then if you want to throttle one cpu, then you must
> > also throttle the other. 
>    I agree. It is necessary to initiate the state transition for every
> affected cpu. 

thanks,
-Len

Re: [PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Len Brown]

On Monday 28 January 2008 00:55, Zhao Yakui wrote:
> Subject: ACPI : Update the t-state for every affected cpu when t-state is changed
> >From : Zhao Yakui <yakui.zhao@intel.com>
> 
> According to ACPI spec, the _TSD object provides T-state control cross
> logical processor dependency information to OSPM. So the t-state
> coordination should be considered when T-state for one cpu is changed.
> 
> According to ACPI spec, three types of coordination are defined.
> SW_ALL, SW_ANY and HW_ALL.
> SW_ALL: it means that OSPM needs to initiate T-state transition on 
> all processors in the domain. It is necessary to call throttling set function
> for all affected cpus.
> SW_ANY: it means that OSPM may initiate T-state transition on any processor in 
> the domain. 
> HW_ALL: Apec only says that hardware will perform the coordination and doesn't 
> recommend how OSPM coordinate T-state among the affected cpus. So it is treated
> as the type of SW_ALL. It means that OSPM needs to initiate t-state transition
> on all the processors in the domain.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
>  drivers/acpi/processor_throttling.c |   79 +++++++++++++++++++++++++++++++++---
>  1 file changed, 74 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/processor_throttling.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_throttling.c
> +++ linux-2.6/drivers/acpi/processor_throttling.c
> @@ -68,7 +68,7 @@ static int acpi_processor_update_tsd_coo
>  
>  	/*
>  	 * Now that we have _TSD data from all CPUs, lets setup T-state
> -	 * coordination among all CPUs.
> +	 * coordination between all CPUs.
>  	 */
>  	for_each_possible_cpu(i) {
>  		pr = processors[i];
> @@ -988,6 +988,11 @@ int acpi_processor_set_throttling(struct
>  {
>  	cpumask_t saved_mask;
>  	int ret;
> +	unsigned int i;
> +	struct acpi_processor *match_pr;
> +	struct acpi_processor_throttling *p_throttling;
> +	struct throttling_tstate t_state;
> +	cpumask_t online_throttling_cpus;
>  
>  	if (!pr)
>  		return -EINVAL;
> @@ -998,12 +1003,76 @@ int acpi_processor_set_throttling(struct
>  	if ((state < 0) || (state > (pr->throttling.state_count - 1)))
>  		return -EINVAL;
>  
> +	saved_mask = current->cpus_allowed;
> +	t_state.target_state = state;
> +	p_throttling = &(pr->throttling);
> +	cpus_and(online_throttling_cpus, cpu_online_map,
> +			p_throttling->shared_cpu_map);
>  	/*
> -	 * Migrate task to the cpu pointed by pr.
> +	 * The throttling notifier will be called for every
> +	 * affected cpu in order to get one proper T-state.
> +	 * The notifier event is THROTTLING_PRECHANGE.
>  	 */
> -	saved_mask = current->cpus_allowed;
> -	set_cpus_allowed(current, cpumask_of_cpu(pr->id));
> -	ret = pr->throttling.acpi_processor_set_throttling(pr, state);

drivers/acpi/processor_throttling.c:990: warning: ‘ret’ may be used uninitialized in this function

-Len

> +	for_each_cpu_mask(i, online_throttling_cpus) {
> +		t_state.cpu = i;
> +		acpi_processor_throttling_notifier(THROTTLING_PRECHANGE,
> +							&t_state);
> +	}
> +	/*
> +	 * The function of acpi_processor_set_throttling will be called
> +	 * to switch T-state. If the coordination type is SW_ALL or HW_ALL,
> +	 * it is necessary to call it for every affected cpu. Otherwise
> +	 * it can be called only for the cpu pointed by pr.
> +	 */
> +	if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) {
> +		set_cpus_allowed(current, cpumask_of_cpu(pr->id));
> +		ret = p_throttling->acpi_processor_set_throttling(pr,
> +						t_state.target_state);
> +	} else {
> +		/*
> +		 * When the T-state coordination is SW_ALL or HW_ALL,
> +		 * it is necessary to set T-state for every affected
> +		 * cpus.
> +		 */
> +		for_each_cpu_mask(i, online_throttling_cpus) {
> +			match_pr = processors[i];
> +			/*
> +			 * If the pointer is invalid, we will report the
> +			 * error message and continue.
> +			 */
> +			if (!match_pr) {
> +				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +					"Invalid Pointer for CPU %d\n", i));
> +				continue;
> +			}
> +			/*
> +			 * If the throttling control is unsupported on CPU i,
> +			 * we will report the error message and continue.
> +			 */
> +			if (!match_pr->flags.throttling) {
> +				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +					"Throttling Controll is unsupported "
> +					"on CPU %d\n", i));
> +				continue;
> +			}
> +			t_state.cpu = i;
> +			set_cpus_allowed(current, cpumask_of_cpu(i));
> +			ret = match_pr->throttling.
> +				acpi_processor_set_throttling(
> +				match_pr, t_state.target_state);
> +		}
> +	}
> +	/*
> +	 * After the set_throttling is called, the
> +	 * throttling notifier is called for every
> +	 * affected cpu to update the T-states.
> +	 * The notifier event is THROTTLING_POSTCHANGE
> +	 */
> +	for_each_cpu_mask(i, online_throttling_cpus) {
> +		t_state.cpu = i;
> +		acpi_processor_throttling_notifier(THROTTLING_POSTCHANGE,
> +							&t_state);
> +	}
>  	/* restore the previous state */
>  	set_cpus_allowed(current, saved_mask);
>  	return ret;
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 5/5] ACPI: Update the t-state for every affected cpu when t-state is changed

[Zhao, Yakui]

drivers/acpi/processor_throttling.c:990: warning: 'ret' may be used
uninitialized in this function

-Len
I will eliminate the warning message and refresh the patch. 
Thanks.

> +	for_each_cpu_mask(i, online_throttling_cpus) {
> +		t_state.cpu = i;
> +		acpi_processor_throttling_notifier(THROTTLING_PRECHANGE,
> +							&t_state);
> +	}
> +	/*
> +	 * The function of acpi_processor_set_throttling will be called
> +	 * to switch T-state. If the coordination type is SW_ALL or
HW_ALL,
> +	 * it is necessary to call it for every affected cpu. Otherwise
> +	 * it can be called only for the cpu pointed by pr.
> +	 */
> +	if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) {
> +		set_cpus_allowed(current, cpumask_of_cpu(pr->id));
> +		ret = p_throttling->acpi_processor_set_throttling(pr,
> +						t_state.target_state);
> +	} else {
> +		/*
> +		 * When the T-state coordination is SW_ALL or HW_ALL,
> +		 * it is necessary to set T-state for every affected
> +		 * cpus.
> +		 */
> +		for_each_cpu_mask(i, online_throttling_cpus) {
> +			match_pr = processors[i];
> +			/*
> +			 * If the pointer is invalid, we will report the
> +			 * error message and continue.
> +			 */
> +			if (!match_pr) {
> +				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +					"Invalid Pointer for CPU %d\n",
i));
> +				continue;
> +			}
> +			/*
> +			 * If the throttling control is unsupported on
CPU i,
> +			 * we will report the error message and
continue.
> +			 */
> +			if (!match_pr->flags.throttling) {
> +				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +					"Throttling Controll is
unsupported "
> +					"on CPU %d\n", i));
> +				continue;
> +			}
> +			t_state.cpu = i;
> +			set_cpus_allowed(current, cpumask_of_cpu(i));
> +			ret = match_pr->throttling.
> +				acpi_processor_set_throttling(
> +				match_pr, t_state.target_state);
> +		}
> +	}
> +	/*
> +	 * After the set_throttling is called, the
> +	 * throttling notifier is called for every
> +	 * affected cpu to update the T-states.
> +	 * The notifier event is THROTTLING_POSTCHANGE
> +	 */
> +	for_each_cpu_mask(i, online_throttling_cpus) {
> +		t_state.cpu = i;
> +
acpi_processor_throttling_notifier(THROTTLING_POSTCHANGE,
> +							&t_state);
> +	}
>  	/* restore the previous state */
>  	set_cpus_allowed(current, saved_mask);
>  	return ret;
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>