| Message ID | 20230826095743.1138495-1-liaochang1@huawei.com |
|---|---|
| State | Accepted |
| Commit | 4c2fdf7393647a7b01a83f49c4a331d562016640 |
| Headers | show |
| Series | cpufreq: pcc: Fix the potentinal scheduling delays in target_index() | expand |
On 26-08-23, 09:57, Liao Chang wrote: > pcc_cpufreq_target(): > cpufreq_freq_transition_begin(); > spin_lock(&pcc_lock); > [critical section] > cpufreq_freq_transition_end(); > spin_unlock(&pcc_lock); > > Above code has a performance issue, consider that Task0 executes > 'cpufreq_freq_transition_end()' to wake Task1 and preempted imediatedly > without releasing 'pcc_lock', then Task1 needs to wait for Task0 to > release 'pcc_lock'. In the worst case, this locking order can result in > Task1 wasting two scheduling rounds before it can enter the critical > section. > > Signed-off-by: Liao Chang <liaochang1@huawei.com> > --- > drivers/cpufreq/pcc-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c > index 73efbcf5513b..9d732a00e2a5 100644 > --- a/drivers/cpufreq/pcc-cpufreq.c > +++ b/drivers/cpufreq/pcc-cpufreq.c > @@ -232,8 +232,8 @@ static int pcc_cpufreq_target(struct cpufreq_policy *policy, > status = ioread16(&pcch_hdr->status); > iowrite16(0, &pcch_hdr->status); > > - cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); > spin_unlock(&pcc_lock); > + cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); > > if (status != CMD_COMPLETE) { > pr_debug("target: FAILED for cpu %d, with status: 0x%x\n", Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Mon, Aug 28, 2023 at 9:02 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 26-08-23, 09:57, Liao Chang wrote: > > pcc_cpufreq_target(): > > cpufreq_freq_transition_begin(); > > spin_lock(&pcc_lock); > > [critical section] > > cpufreq_freq_transition_end(); > > spin_unlock(&pcc_lock); > > > > Above code has a performance issue, consider that Task0 executes > > 'cpufreq_freq_transition_end()' to wake Task1 and preempted imediatedly > > without releasing 'pcc_lock', then Task1 needs to wait for Task0 to > > release 'pcc_lock'. In the worst case, this locking order can result in > > Task1 wasting two scheduling rounds before it can enter the critical > > section. > > > > Signed-off-by: Liao Chang <liaochang1@huawei.com> > > --- > > drivers/cpufreq/pcc-cpufreq.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c > > index 73efbcf5513b..9d732a00e2a5 100644 > > --- a/drivers/cpufreq/pcc-cpufreq.c > > +++ b/drivers/cpufreq/pcc-cpufreq.c > > @@ -232,8 +232,8 @@ static int pcc_cpufreq_target(struct cpufreq_policy *policy, > > status = ioread16(&pcch_hdr->status); > > iowrite16(0, &pcch_hdr->status); > > > > - cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); > > spin_unlock(&pcc_lock); > > + cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); > > > > if (status != CMD_COMPLETE) { > > pr_debug("target: FAILED for cpu %d, with status: 0x%x\n", > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> Applied as 6.6-rc material, thanks!
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c index 73efbcf5513b..9d732a00e2a5 100644 --- a/drivers/cpufreq/pcc-cpufreq.c +++ b/drivers/cpufreq/pcc-cpufreq.c @@ -232,8 +232,8 @@ static int pcc_cpufreq_target(struct cpufreq_policy *policy, status = ioread16(&pcch_hdr->status); iowrite16(0, &pcch_hdr->status); - cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); spin_unlock(&pcc_lock); + cpufreq_freq_transition_end(policy, &freqs, status != CMD_COMPLETE); if (status != CMD_COMPLETE) { pr_debug("target: FAILED for cpu %d, with status: 0x%x\n",
pcc_cpufreq_target(): cpufreq_freq_transition_begin(); spin_lock(&pcc_lock); [critical section] cpufreq_freq_transition_end(); spin_unlock(&pcc_lock); Above code has a performance issue, consider that Task0 executes 'cpufreq_freq_transition_end()' to wake Task1 and preempted imediatedly without releasing 'pcc_lock', then Task1 needs to wait for Task0 to release 'pcc_lock'. In the worst case, this locking order can result in Task1 wasting two scheduling rounds before it can enter the critical section. Signed-off-by: Liao Chang <liaochang1@huawei.com> --- drivers/cpufreq/pcc-cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)