[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Xiao Yang]

If intel_pstate driver has been initialized but turbo is disabled by BIOS
or unavailable on processor(i.e. intel_pstate/no_turbo file exists but its
default value is 1), we cannot write data into intel_pstate/no_turbo and
return EPERM, as below:
------------------------------------------------------------------------
cpufreq_boost    1  TBROK  :  safe_file_ops.c:301: Failed to close FILE '/sys/devices/system/cpu/intel_pstate/no_turbo' at cpufreq_boost.c:151: errno=EPERM(1): Operation not permitted
cpufreq_boost    2  TBROK  :  safe_file_ops.c:301: Remaining cases broken
------------------------------------------------------------------------

We try to skip test in this case.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 .../kernel/device-drivers/cpufreq/cpufreq_boost.c  | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/testcases/kernel/device-drivers/cpufreq/cpufreq_boost.c b/testcases/kernel/device-drivers/cpufreq/cpufreq_boost.c
index e345821..802461b 100644
--- a/testcases/kernel/device-drivers/cpufreq/cpufreq_boost.c
+++ b/testcases/kernel/device-drivers/cpufreq/cpufreq_boost.c
@@ -28,6 +28,7 @@
  */
 
 #define _GNU_SOURCE
+#include <errno.h>
 #include <sched.h>
 #include <time.h>
 
@@ -59,6 +60,22 @@ static char governor_name[16];
 
 const char maxspeed[]	= SYSFS_CPU_DIR "cpu0/cpufreq/scaling_max_freq";
 
+static void check_set_turbo(char *file, char *off)
+{
+	int fd;
+
+	fd = SAFE_OPEN(NULL, file, O_WRONLY);
+
+	/* We try to skip test when getting EPERM. */
+	if (write(fd, off, 1) == -1 && errno == EPERM) {
+		SAFE_CLOSE(NULL, fd);
+		tst_brkm(TCONF, NULL, "Turbo is disabled by "
+			 "BIOS or unavailable on processor");
+	}
+
+	SAFE_CLOSE(NULL, fd);
+}
+
 static void cleanup(void)
 {
 	FILE_PRINTF(cdrv[id].file, "%d", boost_value);
@@ -93,6 +110,13 @@ static void setup(void)
 
 	SAFE_FILE_SCANF(NULL, cdrv[i].file, "%d", &boost_value);
 
+	/* For intel_pstate, we cannot write data into intel_pstate/no_turbo
+	 * and return EPERM if turbo is disabled by BIOS or unavailable on
+	 * processor.  We should check this case by writing original data.
+	 */
+	if (!strcmp(cdrv[i].name, "intel_pstate") && boost_value == cdrv[i].off)
+		check_set_turbo(cdrv[i].file, cdrv[i].off_str);
+
 	/* change cpu0 scaling governor */
 	SAFE_FILE_SCANF(NULL, governor, "%s", governor_name);
 	SAFE_FILE_PRINTF(cleanup, governor, "%s", "performance");
-- 
1.8.3.1

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Petr Vorel]

Hi Xiao,

> If intel_pstate driver has been initialized but turbo is disabled by BIOS
> or unavailable on processor(i.e. intel_pstate/no_turbo file exists but its
> default value is 1), we cannot write data into intel_pstate/no_turbo and
> return EPERM, as below:
> ------------------------------------------------------------------------
> cpufreq_boost    1  TBROK  :  safe_file_ops.c:301: Failed to close FILE '/sys/devices/system/cpu/intel_pstate/no_turbo' at cpufreq_boost.c:151: errno=EPERM(1): Operation not permitted
> cpufreq_boost    2  TBROK  :  safe_file_ops.c:301: Remaining cases broken
> ------------------------------------------------------------------------

> We try to skip test in this case.

> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Acked-by: Petr Vorel <pvorel@suse.cz>

Not merging it as it's a git freeze (although it's a fix).

> +	/* We try to skip test when getting EPERM. */
> +	if (write(fd, off, 1) == -1 && errno == EPERM) {
> +		SAFE_CLOSE(NULL, fd);
> +		tst_brkm(TCONF, NULL, "Turbo is disabled by "
> +			 "BIOS or unavailable on processor");
Minor tip: I wouldn't split the string (better for grep, also checkpatch.pl
warns about it).

Kind regards,
Petr

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Xiao Yang]

On 2019/01/08 17:33, Petr Vorel wrote:
> Hi Xiao,
>
>> If intel_pstate driver has been initialized but turbo is disabled by BIOS
>> or unavailable on processor(i.e. intel_pstate/no_turbo file exists but its
>> default value is 1), we cannot write data into intel_pstate/no_turbo and
>> return EPERM, as below:
>> ------------------------------------------------------------------------
>> cpufreq_boost    1  TBROK  :  safe_file_ops.c:301: Failed to close FILE '/sys/devices/system/cpu/intel_pstate/no_turbo' at cpufreq_boost.c:151: errno=EPERM(1): Operation not permitted
>> cpufreq_boost    2  TBROK  :  safe_file_ops.c:301: Remaining cases broken
>> ------------------------------------------------------------------------
>> We try to skip test in this case.
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> Acked-by: Petr Vorel<pvorel@suse.cz>
>
> Not merging it as it's a git freeze (although it's a fix).
>
>> +	/* We try to skip test when getting EPERM. */
>> +	if (write(fd, off, 1) == -1&&  errno == EPERM) {
>> +		SAFE_CLOSE(NULL, fd);
>> +		tst_brkm(TCONF, NULL, "Turbo is disabled by "
>> +			 "BIOS or unavailable on processor");
> Minor tip: I wouldn't split the string (better for grep, also checkpatch.pl
> warns about it).
Hi Petr,

Thanks for your review.

1) I will get the "line over 80 characters" warning if i don't split the string.
2) I will get the "quoted string split across lines" warning if i split the string.

Which one should i choose?  I am not sure.

Best Regards,
Xiao Yang

> Kind regards,
> Petr
>
>
> .
>

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Cristian Marussi]

Hi

On 08/01/2019 09:46, Xiao Yang wrote:
> On 2019/01/08 17:33, Petr Vorel wrote:
>> Hi Xiao,
>>
>>> If intel_pstate driver has been initialized but turbo is disabled by BIOS
>>> or unavailable on processor(i.e. intel_pstate/no_turbo file exists but its
>>> default value is 1), we cannot write data into intel_pstate/no_turbo and
>>> return EPERM, as below:
>>> ------------------------------------------------------------------------
>>> cpufreq_boost    1  TBROK  :  safe_file_ops.c:301: Failed to close FILE '/sys/devices/system/cpu/intel_pstate/no_turbo' at cpufreq_boost.c:151: errno=EPERM(1): Operation not permitted
>>> cpufreq_boost    2  TBROK  :  safe_file_ops.c:301: Remaining cases broken
>>> ------------------------------------------------------------------------
>>> We try to skip test in this case.
>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> Acked-by: Petr Vorel<pvorel@suse.cz>
>>
>> Not merging it as it's a git freeze (although it's a fix).
>>
>>> +	/* We try to skip test when getting EPERM. */
>>> +	if (write(fd, off, 1) == -1&&  errno == EPERM) {
>>> +		SAFE_CLOSE(NULL, fd);
>>> +		tst_brkm(TCONF, NULL, "Turbo is disabled by "
>>> +			 "BIOS or unavailable on processor");
>> Minor tip: I wouldn't split the string (better for grep, also checkpatch.pl
>> warns about it).
> Hi Petr,
> 
> Thanks for your review.
> 
> 1) I will get the "line over 80 characters" warning if i don't split the string.
> 2) I will get the "quoted string split across lines" warning if i split the string.
> 
> Which one should i choose?  I am not sure.

I could be wrong but i think that if you keep the string unsplitted and move it
on its own line checkpatch.pl should be fine, like in:

tst_brkm(TCONF, NULL,
         "Turbo is disabled by BIOS or unavailable on processor");

Regards

Cristian

> 
> Best Regards,
> Xiao Yang
> 
>> Kind regards,
>> Petr
>>
>>
>> .
>>
> 
> 
> 
> 

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Xiao Yang]

On 2019/01/08 18:10, Cristian Marussi wrote:
> Hi
>
> On 08/01/2019 09:46, Xiao Yang wrote:
>> On 2019/01/08 17:33, Petr Vorel wrote:
>>> Hi Xiao,
>>>
>>>> If intel_pstate driver has been initialized but turbo is disabled by BIOS
>>>> or unavailable on processor(i.e. intel_pstate/no_turbo file exists but its
>>>> default value is 1), we cannot write data into intel_pstate/no_turbo and
>>>> return EPERM, as below:
>>>> ------------------------------------------------------------------------
>>>> cpufreq_boost    1  TBROK  :  safe_file_ops.c:301: Failed to close FILE '/sys/devices/system/cpu/intel_pstate/no_turbo' at cpufreq_boost.c:151: errno=EPERM(1): Operation not permitted
>>>> cpufreq_boost    2  TBROK  :  safe_file_ops.c:301: Remaining cases broken
>>>> ------------------------------------------------------------------------
>>>> We try to skip test in this case.
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>> Acked-by: Petr Vorel<pvorel@suse.cz>
>>>
>>> Not merging it as it's a git freeze (although it's a fix).
>>>
>>>> +	/* We try to skip test when getting EPERM. */
>>>> +	if (write(fd, off, 1) == -1&&   errno == EPERM) {
>>>> +		SAFE_CLOSE(NULL, fd);
>>>> +		tst_brkm(TCONF, NULL, "Turbo is disabled by "
>>>> +			 "BIOS or unavailable on processor");
>>> Minor tip: I wouldn't split the string (better for grep, also checkpatch.pl
>>> warns about it).
>> Hi Petr,
>>
>> Thanks for your review.
>>
>> 1) I will get the "line over 80 characters" warning if i don't split the string.
>> 2) I will get the "quoted string split across lines" warning if i split the string.
>>
>> Which one should i choose?  I am not sure.
> I could be wrong but i think that if you keep the string unsplitted and move it
> on its own line checkpatch.pl should be fine, like in:
>
> tst_brkm(TCONF, NULL,
>           "Turbo is disabled by BIOS or unavailable on processor");
Hi Cristian, Petr

Right, it's my fault.  According to checkpatch.pl code, one line with a single string
doesn't trigger the "line over 80 characters" warning even if its length over 80 chars.

Best Regards,
Xiao Yang

> Regards
>
> Cristian
>
>> Best Regards,
>> Xiao Yang
>>
>>> Kind regards,
>>> Petr
>>>
>>>
>>> .
>>>
>>
>>
>>
>

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Petr Vorel]

Hi Xiao, Cristian,

> I could be wrong but i think that if you keep the string unsplitted and move it
> on its own line checkpatch.pl should be fine, like in:

> tst_brkm(TCONF, NULL,
>          "Turbo is disabled by BIOS or unavailable on processor");
Yes.

Xiao, no need to repost patch, this can be done during merge.


Kind regards,
Petr

[LTP] [PATCH] device-drivers/cpufreq_boost.c: skip test if turbo is disabled by BIOS or unavailable on processor

[Petr Vorel]

Hi Xiao,

> > I could be wrong but i think that if you keep the string unsplitted and move it
> > on its own line checkpatch.pl should be fine, like in:

> > tst_brkm(TCONF, NULL,
> >          "Turbo is disabled by BIOS or unavailable on processor");
> Yes.

> Xiao, no need to repost patch, this can be done during merge.
Merged with unsplitted string change.

Thanks for your work.


Kind regards,
Petr