libs/power: fix the resource leaking issue
Checks
Commit Message
Fixes: e6c6dc0f96c8 ("power: add p-state driver compatibility")
Coverity issue: 328528
Also add the missing functionality of enable/disable turbo
Signed-off-by: Liang Ma <liang.j.ma@intel.com>
---
lib/librte_power/power_pstate_cpufreq.c | 34 ++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
Comments
> -----Original Message-----
> From: Ma, Liang J
> Sent: Friday, December 28, 2018 7:33 PM
> To: Hunt, David <david.hunt@intel.com>
> Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>; Yao, Lei
> A <lei.a.yao@intel.com>; Ma, Liang J <liang.j.ma@intel.com>
> Subject: [PATCH] libs/power: fix the resource leaking issue
>
> Fixes: e6c6dc0f96c8 ("power: add p-state driver compatibility")
> Coverity issue: 328528
>
> Also add the missing functionality of enable/disable turbo
>
> Signed-off-by: Liang Ma <liang.j.ma@intel.com>
Reviewed-by: Lei Yao <lei.a.yao@intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
This patch has been tested based on 19.02-rc1 code.
> ---
> lib/librte_power/power_pstate_cpufreq.c | 34
> ++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_power/power_pstate_cpufreq.c
> b/lib/librte_power/power_pstate_cpufreq.c
> index 411d0eb..cb226a5 100644
> --- a/lib/librte_power/power_pstate_cpufreq.c
> +++ b/lib/librte_power/power_pstate_cpufreq.c
> @@ -160,6 +160,10 @@ power_init_for_setting_freq(struct
> pstate_power_info *pi)
> pi->lcore_id);
>
> f_max = fopen(fullpath_max, "rw+");
> +
> + if (f_max == NULL)
> + fclose(f_min);
> +
> FOPEN_OR_ERR_RET(f_max, -1);
>
> pi->f_cur_min = f_min;
> @@ -214,7 +218,13 @@ set_freq_internal(struct pstate_power_info *pi,
> uint32_t idx)
> /* Turbo is available and enabled, first freq bucket is sys max freq */
> if (pi->turbo_available && pi->turbo_enable && (idx == 0))
> target_freq = pi->sys_max_freq;
> - else
> + else if (pi->turbo_available && (!pi->turbo_enable) && (idx == 0)) {
> +
> + RTE_LOG(ERR, POWER, "Turbo is off, frequency can't be
> scaled up more %u\n",
> + pi->lcore_id);
> + return -1;
> +
> + } else
> target_freq = pi->freqs[idx];
>
> /* Decrease freq, the min freq should be updated first */
> @@ -394,6 +404,10 @@ power_get_available_freqs(struct
> pstate_power_info *pi)
> FOPEN_OR_ERR_RET(f_min, ret);
>
> f_max = fopen(fullpath_max, "r");
> +
> + if (f_max == NULL)
> + fclose(f_min);
> +
> FOPEN_OR_ERR_RET(f_max, ret);
>
> s_min = fgets(buf_min, sizeof(buf_min), f_min);
> @@ -726,6 +740,14 @@ power_pstate_enable_turbo(unsigned int lcore_id)
> return -1;
> }
>
> + /* Max may have changed, so call to max function */
> + if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
> + RTE_LOG(ERR, POWER,
> + "Failed to set frequency of lcore %u to max\n",
> + lcore_id);
> + return -1;
> + }
> +
> return 0;
> }
>
> @@ -744,6 +766,16 @@ power_pstate_disable_turbo(unsigned int lcore_id)
>
> pi->turbo_enable = 0;
>
> + if ((pi->turbo_available) && (pi->curr_idx <= 1)) {
> + /* Try to set freq to max by default coming out of turbo */
> + if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
> + RTE_LOG(ERR, POWER,
> + "Failed to set frequency of lcore %u to
> max\n",
> + lcore_id);
> + return -1;
> + }
> + }
> +
>
> return 0;
> }
> --
> 2.7.5
28/12/2018 12:32, Liang Ma:
> Fixes: e6c6dc0f96c8 ("power: add p-state driver compatibility")
> Coverity issue: 328528
>
> Also add the missing functionality of enable/disable turbo
If you are changing two things, you probably need to split in two patches.
In 19.02 timeframe, we will get only the fix.
Dave, please could you help?
On 14/1/2019 10:35 PM, Thomas Monjalon wrote:
> 28/12/2018 12:32, Liang Ma:
>> Fixes: e6c6dc0f96c8 ("power: add p-state driver compatibility")
>> Coverity issue: 328528
>>
>> Also add the missing functionality of enable/disable turbo
> If you are changing two things, you probably need to split in two patches.
> In 19.02 timeframe, we will get only the fix.
>
> Dave, please could you help?
>
Sure, Thomas. I've just pushed a v2 which focuses on the resource leak
fix. We can handle the Turbo Boost enhancements in the next release.
Regards,
Dave.
15/01/2019 11:07, Hunt, David:
>
> On 14/1/2019 10:35 PM, Thomas Monjalon wrote:
> > 28/12/2018 12:32, Liang Ma:
> >> Fixes: e6c6dc0f96c8 ("power: add p-state driver compatibility")
> >> Coverity issue: 328528
> >>
> >> Also add the missing functionality of enable/disable turbo
> > If you are changing two things, you probably need to split in two patches.
> > In 19.02 timeframe, we will get only the fix.
> >
> > Dave, please could you help?
> >
>
> Sure, Thomas. I've just pushed a v2 which focuses on the resource leak
> fix. We can handle the Turbo Boost enhancements in the next release.
OK thanks
@@ -160,6 +160,10 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
pi->lcore_id);
f_max = fopen(fullpath_max, "rw+");
+
+ if (f_max == NULL)
+ fclose(f_min);
+
FOPEN_OR_ERR_RET(f_max, -1);
pi->f_cur_min = f_min;
@@ -214,7 +218,13 @@ set_freq_internal(struct pstate_power_info *pi, uint32_t idx)
/* Turbo is available and enabled, first freq bucket is sys max freq */
if (pi->turbo_available && pi->turbo_enable && (idx == 0))
target_freq = pi->sys_max_freq;
- else
+ else if (pi->turbo_available && (!pi->turbo_enable) && (idx == 0)) {
+
+ RTE_LOG(ERR, POWER, "Turbo is off, frequency can't be scaled up more %u\n",
+ pi->lcore_id);
+ return -1;
+
+ } else
target_freq = pi->freqs[idx];
/* Decrease freq, the min freq should be updated first */
@@ -394,6 +404,10 @@ power_get_available_freqs(struct pstate_power_info *pi)
FOPEN_OR_ERR_RET(f_min, ret);
f_max = fopen(fullpath_max, "r");
+
+ if (f_max == NULL)
+ fclose(f_min);
+
FOPEN_OR_ERR_RET(f_max, ret);
s_min = fgets(buf_min, sizeof(buf_min), f_min);
@@ -726,6 +740,14 @@ power_pstate_enable_turbo(unsigned int lcore_id)
return -1;
}
+ /* Max may have changed, so call to max function */
+ if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
+ RTE_LOG(ERR, POWER,
+ "Failed to set frequency of lcore %u to max\n",
+ lcore_id);
+ return -1;
+ }
+
return 0;
}
@@ -744,6 +766,16 @@ power_pstate_disable_turbo(unsigned int lcore_id)
pi->turbo_enable = 0;
+ if ((pi->turbo_available) && (pi->curr_idx <= 1)) {
+ /* Try to set freq to max by default coming out of turbo */
+ if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
+ RTE_LOG(ERR, POWER,
+ "Failed to set frequency of lcore %u to max\n",
+ lcore_id);
+ return -1;
+ }
+ }
+
return 0;
}