[PATCH v2 0/6] Selftests fixes for futex and intel_pstate

[PATCH v2 0/6] Selftests fixes for futex and intel_pstate

[Stafford Horne]

This is followup to my previous patch [1] which was a very simple makefile
change. After a comment from Darren Hart I updated intel_pstate as well,
but noticed a few issues with running intel_pstate. That caused the change
to expand a bit.

 - fix build deps for futex test *original*
 - update makefile for intel_pstate to be in sync with Bamvor's changes
 - update cpupower frequency-info to fix intel_pstate test

For the linux-pm list the change to cpupower might be an issue, but if so
it would be nice if they can suggest a better way to get the self test to
do what it wants to do.

The series is based on Shuah's kselftest next branch, because that is
where Bamvor's latest changes are:
 git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next

[1] https://lkml.org/lkml/2017/1/5/903

Stafford Horne (6):
  selftests/futex: Add stdio used for logging
  selftests/futex: Add headers to makefile dependencies
  cpupower: Restore format of frequency-info limit
  selftests/intel_pstate: Fix warning on loop index overflow
  selftests/intel_pstate: Update makefile to match new style
  selftests: Add intel_pstate to TARGETS

 tools/power/cpupower/utils/cpufreq-info.c         | 21 ++++++++++++---------
 tools/testing/selftests/Makefile                  |  1 +
 tools/testing/selftests/futex/functional/Makefile |  5 ++++-
 tools/testing/selftests/futex/include/logging.h   |  1 +
 tools/testing/selftests/intel_pstate/Makefile     | 13 ++++---------
 tools/testing/selftests/intel_pstate/aperf.c      |  2 +-
 6 files changed, 23 insertions(+), 20 deletions(-)

-- 
2.9.3

[PATCH v2 1/6] selftests/futex: Add stdio used for logging

[Stafford Horne]

Without this I am getting warnings of printf missing when compiling
selftests.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/testing/selftests/futex/include/logging.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/futex/include/logging.h b/tools/testing/selftests/futex/include/logging.h
index 014aa01..e144691 100644
--- a/tools/testing/selftests/futex/include/logging.h
+++ b/tools/testing/selftests/futex/include/logging.h
@@ -21,6 +21,7 @@
 #ifndef _LOGGING_H
 #define _LOGGING_H
 
+#include <stdio.h>
 #include <string.h>
 #include <unistd.h>
 #include <linux/futex.h>
-- 
2.9.3

[PATCH v2 2/6] cpupower: Restore format of frequency-info limit

[Stafford Horne]

The intel_pstate kselftest expects that the output of
`cpupower frequency-info -l | tail -1 | awk ' { print $1 } '`
to get frequency limits.  This does not work after the following two
changes.

 - 562e5f1a3: rework the "cpupower frequency-info" command
   (Jacob Tanenbaum) removed parsable limit output
 - ce512b840: Do not analyse offlined cpus
   (Thomas Renninger) added newline to break limit parsing more

This change preserves human readable output if wanted as well as
parsable output for scripts/tests.

Cc: Thomas Renninger <trenn@suse.com>
Cc: "Shreyas B. Prabhu" <shreyas@linux.vnet.ibm.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/power/cpupower/utils/cpufreq-info.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
index 590d12a..3e701f0 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -285,20 +285,24 @@ static int get_freq_hardware(unsigned int cpu, unsigned int human)
 
 /* --hwlimits / -l */
 
-static int get_hardware_limits(unsigned int cpu)
+static int get_hardware_limits(unsigned int cpu, unsigned int human)
 {
 	unsigned long min, max;
 
-	printf(_("  hardware limits: "));
 	if (cpufreq_get_hardware_limits(cpu, &min, &max)) {
 		printf(_("Not Available\n"));
 		return -EINVAL;
 	}
 
-	print_speed(min);
-	printf(" - ");
-	print_speed(max);
-	printf("\n");
+	if (human) {
+		printf(_("  hardware limits: "));
+		print_speed(min);
+		printf(" - ");
+		print_speed(max);
+		printf("\n");
+	} else {
+		printf("%lu %lu\n", min, max);
+	}
 	return 0;
 }
 
@@ -456,7 +460,7 @@ static void debug_output_one(unsigned int cpu)
 	get_related_cpus(cpu);
 	get_affected_cpus(cpu);
 	get_latency(cpu, 1);
-	get_hardware_limits(cpu);
+	get_hardware_limits(cpu, 1);
 
 	freqs = cpufreq_get_available_frequencies(cpu);
 	if (freqs) {
@@ -622,7 +626,7 @@ int cmd_freq_info(int argc, char **argv)
 			ret = get_driver(cpu);
 			break;
 		case 'l':
-			ret = get_hardware_limits(cpu);
+			ret = get_hardware_limits(cpu, human);
 			break;
 		case 'w':
 			ret = get_freq_hardware(cpu, human);
@@ -639,7 +643,6 @@ int cmd_freq_info(int argc, char **argv)
 		}
 		if (ret)
 			return ret;
-		printf("\n");
 	}
 	return ret;
 }
-- 
2.9.3

[PATCH v2 2/6] selftests/futex: Add headers to makefile dependencies

[Stafford Horne]

I am working on doing selftests for openrisc and found issues with the
futex test not building after changes to the tests source.

The futex makefile did not contain dependencies for all headers, so if
we make changes to logging.h rebuild will not happen. Add headers to
fix it up.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/testing/selftests/futex/functional/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index ac35782..a648e7a 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -2,7 +2,10 @@ INCLUDES := -I../include -I../../
 CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES)
 LDFLAGS := $(LDFLAGS) -pthread -lrt
 
-HEADERS := ../include/futextest.h
+HEADERS := \
+	../include/futextest.h \
+	../include/atomic.h \
+	../include/logging.h
 TEST_GEN_FILES := \
 	futex_wait_timeout \
 	futex_wait_wouldblock \
-- 
2.9.3

[PATCH v2 3/6] cpupower: Restore format of frequency-info limit

[Stafford Horne]

The intel_pstate kselftest expects that the output of
`cpupower frequency-info -l | tail -1 | awk ' { print $1 } '`
to get frequency limits.  This does not work after the following two
changes.

 - 562e5f1a3: rework the "cpupower frequency-info" command
   (Jacob Tanenbaum) removed parsable limit output
 - ce512b840: Do not analyse offlined cpus
   (Thomas Renninger) added newline to break limit parsing more

This change preserves human readable output if wanted as well as
parsable output for scripts/tests.

Cc: Thomas Renninger <trenn@suse.com>
Cc: "Shreyas B. Prabhu" <shreyas@linux.vnet.ibm.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/power/cpupower/utils/cpufreq-info.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
index 590d12a..3e701f0 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -285,20 +285,24 @@ static int get_freq_hardware(unsigned int cpu, unsigned int human)
 
 /* --hwlimits / -l */
 
-static int get_hardware_limits(unsigned int cpu)
+static int get_hardware_limits(unsigned int cpu, unsigned int human)
 {
 	unsigned long min, max;
 
-	printf(_("  hardware limits: "));
 	if (cpufreq_get_hardware_limits(cpu, &min, &max)) {
 		printf(_("Not Available\n"));
 		return -EINVAL;
 	}
 
-	print_speed(min);
-	printf(" - ");
-	print_speed(max);
-	printf("\n");
+	if (human) {
+		printf(_("  hardware limits: "));
+		print_speed(min);
+		printf(" - ");
+		print_speed(max);
+		printf("\n");
+	} else {
+		printf("%lu %lu\n", min, max);
+	}
 	return 0;
 }
 
@@ -456,7 +460,7 @@ static void debug_output_one(unsigned int cpu)
 	get_related_cpus(cpu);
 	get_affected_cpus(cpu);
 	get_latency(cpu, 1);
-	get_hardware_limits(cpu);
+	get_hardware_limits(cpu, 1);
 
 	freqs = cpufreq_get_available_frequencies(cpu);
 	if (freqs) {
@@ -622,7 +626,7 @@ int cmd_freq_info(int argc, char **argv)
 			ret = get_driver(cpu);
 			break;
 		case 'l':
-			ret = get_hardware_limits(cpu);
+			ret = get_hardware_limits(cpu, human);
 			break;
 		case 'w':
 			ret = get_freq_hardware(cpu, human);
@@ -639,7 +643,6 @@ int cmd_freq_info(int argc, char **argv)
 		}
 		if (ret)
 			return ret;
-		printf("\n");
 	}
 	return ret;
 }
-- 
2.9.3

[PATCH v2 4/6] selftests/intel_pstate: Fix warning on loop index overflow

[Stafford Horne]

The build was showing the warning:
 aperf.c:60:27: warning: iteration 2147483647 invokes undefined behavior
 [-Waggressive-loop-optimizations]
  for (i=0; i<0x8fffffff; i++) {

This change sets i, cpu and fd to unsigned int as they should not need
to be signed.

Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/testing/selftests/intel_pstate/aperf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/intel_pstate/aperf.c b/tools/testing/selftests/intel_pstate/aperf.c
index 6046e18..cd72f3d 100644
--- a/tools/testing/selftests/intel_pstate/aperf.c
+++ b/tools/testing/selftests/intel_pstate/aperf.c
@@ -14,7 +14,7 @@ void usage(char *name) {
 }
 
 int main(int argc, char **argv) {
-	int i, cpu, fd;
+	unsigned int i, cpu, fd;
 	char msr_file_name[64];
 	long long tsc, old_tsc, new_tsc;
 	long long aperf, old_aperf, new_aperf;
-- 
2.9.3

[PATCH v2 5/6] selftests/intel_pstate: Update makefile to match new style

[Stafford Horne]

Recent changes from Bamvor (88baa78d1f318) have standardized the
variable names like TEST_GEN_FILES and removed the need for make targets
all and clean.

These changes bring the intel_pstate test inline with those changes.

Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/testing/selftests/intel_pstate/Makefile | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/intel_pstate/Makefile b/tools/testing/selftests/intel_pstate/Makefile
index f5f1a28..19678e9 100644
--- a/tools/testing/selftests/intel_pstate/Makefile
+++ b/tools/testing/selftests/intel_pstate/Makefile
@@ -1,15 +1,10 @@
-CC := $(CROSS_COMPILE)gcc
 CFLAGS := $(CFLAGS) -Wall -D_GNU_SOURCE
 LDFLAGS := $(LDFLAGS) -lm
 
-TARGETS := msr aperf
+TEST_GEN_FILES := msr aperf
 
-TEST_PROGS := $(TARGETS) run.sh
+TEST_PROGS := run.sh
 
-.PHONY: all clean
-all: $(TARGETS)
+include ../lib.mk
 
-$(TARGETS): $(HEADERS)
-
-clean:
-	rm -f $(TARGETS)
+$(TEST_GEN_FILES): $(HEADERS)
-- 
2.9.3

[PATCH v2 6/6] selftests: Add intel_pstate to TARGETS

[Stafford Horne]

This test was missing from the TARGETS list.  The test requires patches
to cpupower to pass correctly.

Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 tools/testing/selftests/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2741f9e..c6ccf57 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -8,6 +8,7 @@ TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
 TARGETS += gpio
+TARGETS += intel_pstate
 TARGETS += ipc
 TARGETS += kcmp
 TARGETS += lib
-- 
2.9.3

Re: [PATCH v2 2/6] cpupower: Restore format of frequency-info limit

[Stafford Horne]

Hi All,

Please ignore this, its a dup of the patch with the same name.

I had an slight issue with reordering patches and this got send out.

On Thu, Jan 12, 2017 at 12:11:15AM +0900, Stafford Horne wrote:
> The intel_pstate kselftest expects that the output of
> `cpupower frequency-info -l | tail -1 | awk ' { print $1 } '`
> to get frequency limits.  This does not work after the following two
> changes.
> 
>  - 562e5f1a3: rework the "cpupower frequency-info" command
>    (Jacob Tanenbaum) removed parsable limit output
>  - ce512b840: Do not analyse offlined cpus
>    (Thomas Renninger) added newline to break limit parsing more
> 
> This change preserves human readable output if wanted as well as
> parsable output for scripts/tests.
> 
> Cc: Thomas Renninger <trenn@suse.com>
> Cc: "Shreyas B. Prabhu" <shreyas@linux.vnet.ibm.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  tools/power/cpupower/utils/cpufreq-info.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 590d12a..3e701f0 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -285,20 +285,24 @@ static int get_freq_hardware(unsigned int cpu, unsigned int human)
>  
>  /* --hwlimits / -l */
>  
> -static int get_hardware_limits(unsigned int cpu)
> +static int get_hardware_limits(unsigned int cpu, unsigned int human)
>  {
>  	unsigned long min, max;
>  
> -	printf(_("  hardware limits: "));
>  	if (cpufreq_get_hardware_limits(cpu, &min, &max)) {
>  		printf(_("Not Available\n"));
>  		return -EINVAL;
>  	}
>  
> -	print_speed(min);
> -	printf(" - ");
> -	print_speed(max);
> -	printf("\n");
> +	if (human) {
> +		printf(_("  hardware limits: "));
> +		print_speed(min);
> +		printf(" - ");
> +		print_speed(max);
> +		printf("\n");
> +	} else {
> +		printf("%lu %lu\n", min, max);
> +	}
>  	return 0;
>  }
>  
> @@ -456,7 +460,7 @@ static void debug_output_one(unsigned int cpu)
>  	get_related_cpus(cpu);
>  	get_affected_cpus(cpu);
>  	get_latency(cpu, 1);
> -	get_hardware_limits(cpu);
> +	get_hardware_limits(cpu, 1);
>  
>  	freqs = cpufreq_get_available_frequencies(cpu);
>  	if (freqs) {
> @@ -622,7 +626,7 @@ int cmd_freq_info(int argc, char **argv)
>  			ret = get_driver(cpu);
>  			break;
>  		case 'l':
> -			ret = get_hardware_limits(cpu);
> +			ret = get_hardware_limits(cpu, human);
>  			break;
>  		case 'w':
>  			ret = get_freq_hardware(cpu, human);
> @@ -639,7 +643,6 @@ int cmd_freq_info(int argc, char **argv)
>  		}
>  		if (ret)
>  			return ret;
> -		printf("\n");
>  	}
>  	return ret;
>  }
> -- 
> 2.9.3
> 

Re: [PATCH v2 0/6] Selftests fixes for futex and intel_pstate

[Shuah Khan]

On 01/11/2017 08:11 AM, Stafford Horne wrote:
> This is followup to my previous patch [1] which was a very simple makefile
> change. After a comment from Darren Hart I updated intel_pstate as well,
> but noticed a few issues with running intel_pstate. That caused the change
> to expand a bit.
> 
>  - fix build deps for futex test *original*
>  - update makefile for intel_pstate to be in sync with Bamvor's changes
>  - update cpupower frequency-info to fix intel_pstate test
> 
> For the linux-pm list the change to cpupower might be an issue, but if so
> it would be nice if they can suggest a better way to get the self test to
> do what it wants to do.
> 
> The series is based on Shuah's kselftest next branch, because that is
> where Bamvor's latest changes are:
>  git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
> 
> [1] https://lkml.org/lkml/2017/1/5/903
> 
> Stafford Horne (6):
>   selftests/futex: Add stdio used for logging
>   selftests/futex: Add headers to makefile dependencies
>   cpupower: Restore format of frequency-info limit
>   selftests/intel_pstate: Fix warning on loop index overflow
>   selftests/intel_pstate: Update makefile to match new style
>   selftests: Add intel_pstate to TARGETS
> 
>  tools/power/cpupower/utils/cpufreq-info.c         | 21 ++++++++++++---------
>  tools/testing/selftests/Makefile                  |  1 +
>  tools/testing/selftests/futex/functional/Makefile |  5 ++++-
>  tools/testing/selftests/futex/include/logging.h   |  1 +
>  tools/testing/selftests/intel_pstate/Makefile     | 13 ++++---------
>  tools/testing/selftests/intel_pstate/aperf.c      |  2 +-
>  6 files changed, 23 insertions(+), 20 deletions(-)
> 

The series is now in linux-kselftest next for 4.11

thanks for the fixes.

-- Shuah

Re: [PATCH v2 0/6] Selftests fixes for futex and intel_pstate

[Stafford Horne]

On Thu, Jan 19, 2017 at 10:35:21AM -0700, Shuah Khan wrote:
> On 01/11/2017 08:11 AM, Stafford Horne wrote:
> > This is followup to my previous patch [1] which was a very simple makefile
> > change. After a comment from Darren Hart I updated intel_pstate as well,
> > but noticed a few issues with running intel_pstate. That caused the change
> > to expand a bit.
> > 
> >  - fix build deps for futex test *original*
> >  - update makefile for intel_pstate to be in sync with Bamvor's changes
> >  - update cpupower frequency-info to fix intel_pstate test
> > 
> > For the linux-pm list the change to cpupower might be an issue, but if so
> > it would be nice if they can suggest a better way to get the self test to
> > do what it wants to do.
> > 
> > The series is based on Shuah's kselftest next branch, because that is
> > where Bamvor's latest changes are:
> >  git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
> > 
> > [1] https://lkml.org/lkml/2017/1/5/903
> > 
> > Stafford Horne (6):
> >   selftests/futex: Add stdio used for logging
> >   selftests/futex: Add headers to makefile dependencies
> >   cpupower: Restore format of frequency-info limit
> >   selftests/intel_pstate: Fix warning on loop index overflow
> >   selftests/intel_pstate: Update makefile to match new style
> >   selftests: Add intel_pstate to TARGETS
> > 
> >  tools/power/cpupower/utils/cpufreq-info.c         | 21 ++++++++++++---------
> >  tools/testing/selftests/Makefile                  |  1 +
> >  tools/testing/selftests/futex/functional/Makefile |  5 ++++-
> >  tools/testing/selftests/futex/include/logging.h   |  1 +
> >  tools/testing/selftests/intel_pstate/Makefile     | 13 ++++---------
> >  tools/testing/selftests/intel_pstate/aperf.c      |  2 +-
> >  6 files changed, 23 insertions(+), 20 deletions(-)
> > 
> 
> The series is now in linux-kselftest next for 4.11
> 
> thanks for the fixes.

Thanks,

I see you pulled in the cpupower changes too. We didnt get any reply
even though the maintainers are cc'd. I hope thats no issue, but the
changes seem necessary for maintaining test compatibility.

-Stafford