[PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[Shuah Khan]

This patch series fixes fscanf() ignoring return value warnings.
Consolidates get_run_delay() duplicate defines moving it to
common library.

Shuah Khan (4):
  selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn
  selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return
    warn
  selftests: kvm: move get_run_delay() into lib/test_util
  selftests: kvm: fix get_run_delay() ignoring fscanf() return warn

 .../testing/selftests/kvm/include/test_util.h |  3 +++
 tools/testing/selftests/kvm/lib/test_util.c   | 22 ++++++++++++++++++-
 tools/testing/selftests/kvm/steal_time.c      | 16 --------------
 .../selftests/kvm/x86_64/mmio_warning_test.c  |  3 ++-
 .../selftests/kvm/x86_64/xen_shinfo_test.c    | 15 -------------
 5 files changed, 26 insertions(+), 33 deletions(-)

-- 
2.30.2

[PATCH 1/4] selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn

[Shuah Khan]

Fix get_warnings_count() to check fscanf() return value to get rid
of the following warning:

x86_64/mmio_warning_test.c: In function ‘get_warnings_count’:
x86_64/mmio_warning_test.c:85:2: warning: ignoring return value of ‘fscanf’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
   85 |  fscanf(f, "%d", &warnings);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/kvm/x86_64/mmio_warning_test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
index e6480fd5c4bd..8039e1eff938 100644
--- a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
+++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
@@ -82,7 +82,8 @@ int get_warnings_count(void)
 	FILE *f;
 
 	f = popen("dmesg | grep \"WARNING:\" | wc -l", "r");
-	fscanf(f, "%d", &warnings);
+	if (fscanf(f, "%d", &warnings) < 1)
+		warnings = 0;
 	fclose(f);
 
 	return warnings;
-- 
2.30.2

[PATCH 2/4] selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return warn

[Shuah Khan]

Fix get_trans_hugepagesz() to check fscanf() return value to get rid
of the following warning:

lib/test_util.c: In function ‘get_trans_hugepagesz’:
lib/test_util.c:138:2: warning: ignoring return value of ‘fscanf’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  138 |  fscanf(f, "%ld", &size);
      |  ^~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/kvm/lib/test_util.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index af1031fed97f..938cd423643e 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -129,13 +129,16 @@ size_t get_trans_hugepagesz(void)
 {
 	size_t size;
 	FILE *f;
+	int ret;
 
 	TEST_ASSERT(thp_configured(), "THP is not configured in host kernel");
 
 	f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
 	TEST_ASSERT(f != NULL, "Error in opening transparent_hugepage/hpage_pmd_size");
 
-	fscanf(f, "%ld", &size);
+	ret = fscanf(f, "%ld", &size);
+	ret = fscanf(f, "%ld", &size);
+	TEST_ASSERT(ret < 1, "Error reading transparent_hugepage/hpage_pmd_size");
 	fclose(f);
 
 	return size;
-- 
2.30.2

[PATCH 3/4] selftests: kvm: move get_run_delay() into lib/test_util

[Shuah Khan]

get_run_delay() is defined static in xen_shinfo_test and steal_time test.
Move it to lib and remove code duplication.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/kvm/include/test_util.h   |  1 +
 tools/testing/selftests/kvm/lib/test_util.c       | 15 +++++++++++++++
 tools/testing/selftests/kvm/steal_time.c          | 15 ---------------
 .../selftests/kvm/x86_64/xen_shinfo_test.c        | 15 ---------------
 4 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index d79be15dd3d2..c7409b9b4e5b 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -102,6 +102,7 @@ const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i);
 size_t get_backing_src_pagesz(uint32_t i);
 void backing_src_help(void);
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name);
+long get_run_delay(void);
 
 /*
  * Whether or not the given source type is shared memory (as opposed to
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 938cd423643e..f80dd38a38b2 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <time.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 #include <linux/mman.h>
 #include "linux/kernel.h"
 
@@ -303,3 +304,17 @@ enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name)
 	TEST_FAIL("Unknown backing src type: %s", type_name);
 	return -1;
 }
+
+long get_run_delay(void)
+{
+	char path[64];
+	long val[2];
+	FILE *fp;
+
+	sprintf(path, "/proc/%ld/schedstat", syscall(SYS_gettid));
+	fp = fopen(path, "r");
+	fscanf(fp, "%ld %ld ", &val[0], &val[1]);
+	fclose(fp);
+
+	return val[1];
+}
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index ecec30865a74..51fe95a5c36a 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -10,7 +10,6 @@
 #include <sched.h>
 #include <pthread.h>
 #include <linux/kernel.h>
-#include <sys/syscall.h>
 #include <asm/kvm.h>
 #include <asm/kvm_para.h>
 
@@ -217,20 +216,6 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpuid)
 
 #endif
 
-static long get_run_delay(void)
-{
-	char path[64];
-	long val[2];
-	FILE *fp;
-
-	sprintf(path, "/proc/%ld/schedstat", syscall(SYS_gettid));
-	fp = fopen(path, "r");
-	fscanf(fp, "%ld %ld ", &val[0], &val[1]);
-	fclose(fp);
-
-	return val[1];
-}
-
 static void *do_steal_time(void *arg)
 {
 	struct timespec ts, stop;
diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
index 117bf49a3d79..eda0d2a51224 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
@@ -14,7 +14,6 @@
 #include <stdint.h>
 #include <time.h>
 #include <sched.h>
-#include <sys/syscall.h>
 
 #define VCPU_ID		5
 
@@ -98,20 +97,6 @@ static void guest_code(void)
 	GUEST_DONE();
 }
 
-static long get_run_delay(void)
-{
-        char path[64];
-        long val[2];
-        FILE *fp;
-
-        sprintf(path, "/proc/%ld/schedstat", syscall(SYS_gettid));
-        fp = fopen(path, "r");
-        fscanf(fp, "%ld %ld ", &val[0], &val[1]);
-        fclose(fp);
-
-        return val[1];
-}
-
 static int cmp_timespec(struct timespec *a, struct timespec *b)
 {
 	if (a->tv_sec > b->tv_sec)
-- 
2.30.2

[PATCH 4/4] selftests: kvm: fix get_run_delay() ignoring fscanf() return warn

[Shuah Khan]

Fix get_run_delay() to check fscanf() return value to get rid of the
following warning. When fscanf() fails return MIN_RUN_DELAY_NS from
get_run_delay(). Move MIN_RUN_DELAY_NS from steal_time.c to test_util.h
so get_run_delay() and steal_time.c can use it.

lib/test_util.c: In function ‘get_run_delay’:
lib/test_util.c:316:2: warning: ignoring return value of ‘fscanf’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  316 |  fscanf(fp, "%ld %ld ", &val[0], &val[1]);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/kvm/include/test_util.h | 2 ++
 tools/testing/selftests/kvm/lib/test_util.c     | 4 +++-
 tools/testing/selftests/kvm/steal_time.c        | 1 -
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index c7409b9b4e5b..451fed5ce8e7 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -95,6 +95,8 @@ struct vm_mem_backing_src_alias {
 	uint32_t flag;
 };
 
+#define MIN_RUN_DELAY_NS	200000UL
+
 bool thp_configured(void);
 size_t get_trans_hugepagesz(void);
 size_t get_def_hugetlb_pagesz(void);
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index f80dd38a38b2..a9107bfae402 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -313,7 +313,9 @@ long get_run_delay(void)
 
 	sprintf(path, "/proc/%ld/schedstat", syscall(SYS_gettid));
 	fp = fopen(path, "r");
-	fscanf(fp, "%ld %ld ", &val[0], &val[1]);
+	/* Return MIN_RUN_DELAY_NS upon failure just to be safe */
+	if (fscanf(fp, "%ld %ld ", &val[0], &val[1]) < 2)
+		val[1] = MIN_RUN_DELAY_NS;
 	fclose(fp);
 
 	return val[1];
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index 51fe95a5c36a..2172d65b85e4 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -19,7 +19,6 @@
 
 #define NR_VCPUS		4
 #define ST_GPA_BASE		(1 << 30)
-#define MIN_RUN_DELAY_NS	200000UL
 
 static void *st_gva[NR_VCPUS];
 static uint64_t guest_stolen_time[NR_VCPUS];
-- 
2.30.2

Re: [PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[Paolo Bonzini]

On 15/09/21 23:28, Shuah Khan wrote:
> This patch series fixes fscanf() ignoring return value warnings.
> Consolidates get_run_delay() duplicate defines moving it to
> common library.
> 
> Shuah Khan (4):
>    selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn
>    selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return
>      warn
>    selftests: kvm: move get_run_delay() into lib/test_util
>    selftests: kvm: fix get_run_delay() ignoring fscanf() return warn
> 
>   .../testing/selftests/kvm/include/test_util.h |  3 +++
>   tools/testing/selftests/kvm/lib/test_util.c   | 22 ++++++++++++++++++-
>   tools/testing/selftests/kvm/steal_time.c      | 16 --------------
>   .../selftests/kvm/x86_64/mmio_warning_test.c  |  3 ++-
>   .../selftests/kvm/x86_64/xen_shinfo_test.c    | 15 -------------
>   5 files changed, 26 insertions(+), 33 deletions(-)
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks Shuah!

Paolo

Re: [PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[Shuah Khan]

On 9/15/21 4:34 PM, Paolo Bonzini wrote:
> On 15/09/21 23:28, Shuah Khan wrote:
>> This patch series fixes fscanf() ignoring return value warnings.
>> Consolidates get_run_delay() duplicate defines moving it to
>> common library.
>>
>> Shuah Khan (4):
>>    selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn
>>    selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return
>>      warn
>>    selftests: kvm: move get_run_delay() into lib/test_util
>>    selftests: kvm: fix get_run_delay() ignoring fscanf() return warn
>>
>>   .../testing/selftests/kvm/include/test_util.h |  3 +++
>>   tools/testing/selftests/kvm/lib/test_util.c   | 22 ++++++++++++++++++-
>>   tools/testing/selftests/kvm/steal_time.c      | 16 --------------
>>   .../selftests/kvm/x86_64/mmio_warning_test.c  |  3 ++-
>>   .../selftests/kvm/x86_64/xen_shinfo_test.c    | 15 -------------
>>   5 files changed, 26 insertions(+), 33 deletions(-)
>>
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Thanks Shuah!
> 

Thank you. I can take these through linux-kselftest - let me know
if that causes issues for kvm tree.

thanks,
-- Shuah

Re: [PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[Paolo Bonzini]

On 16/09/21 02:02, Shuah Khan wrote:
> On 9/15/21 4:34 PM, Paolo Bonzini wrote:
>> On 15/09/21 23:28, Shuah Khan wrote:
>>> This patch series fixes fscanf() ignoring return value warnings.
>>> Consolidates get_run_delay() duplicate defines moving it to
>>> common library.
>>>
>>> Shuah Khan (4):
>>>    selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn
>>>    selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return
>>>      warn
>>>    selftests: kvm: move get_run_delay() into lib/test_util
>>>    selftests: kvm: fix get_run_delay() ignoring fscanf() return warn
>>>
>>>   .../testing/selftests/kvm/include/test_util.h |  3 +++
>>>   tools/testing/selftests/kvm/lib/test_util.c   | 22 ++++++++++++++++++-
>>>   tools/testing/selftests/kvm/steal_time.c      | 16 --------------
>>>   .../selftests/kvm/x86_64/mmio_warning_test.c  |  3 ++-
>>>   .../selftests/kvm/x86_64/xen_shinfo_test.c    | 15 -------------
>>>   5 files changed, 26 insertions(+), 33 deletions(-)
>>>
>>
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Thanks Shuah!
>>
> 
> Thank you. I can take these through linux-kselftest - let me know
> if that causes issues for kvm tree.

Go ahead if it's for 5.15-rc, I don't have any selftests patches pending.

Paolo

Re: [PATCH 0/4] selftests: kvm: fscanf warn fixes and cleanups

[Shuah Khan]

On 9/15/21 11:06 PM, Paolo Bonzini wrote:
> On 16/09/21 02:02, Shuah Khan wrote:
>> On 9/15/21 4:34 PM, Paolo Bonzini wrote:
>>> On 15/09/21 23:28, Shuah Khan wrote:
>>>> This patch series fixes fscanf() ignoring return value warnings.
>>>> Consolidates get_run_delay() duplicate defines moving it to
>>>> common library.
>>>>
>>>> Shuah Khan (4):
>>>>    selftests:kvm: fix get_warnings_count() ignoring fscanf() return warn
>>>>    selftests:kvm: fix get_trans_hugepagesz() ignoring fscanf() return
>>>>      warn
>>>>    selftests: kvm: move get_run_delay() into lib/test_util
>>>>    selftests: kvm: fix get_run_delay() ignoring fscanf() return warn
>>>>
>>>>   .../testing/selftests/kvm/include/test_util.h |  3 +++
>>>>   tools/testing/selftests/kvm/lib/test_util.c   | 22 ++++++++++++++++++-
>>>>   tools/testing/selftests/kvm/steal_time.c      | 16 --------------
>>>>   .../selftests/kvm/x86_64/mmio_warning_test.c  |  3 ++-
>>>>   .../selftests/kvm/x86_64/xen_shinfo_test.c    | 15 -------------
>>>>   5 files changed, 26 insertions(+), 33 deletions(-)
>>>>
>>>
>>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> Thanks Shuah!
>>>
>>
>> Thank you. I can take these through linux-kselftest - let me know
>> if that causes issues for kvm tree.
> 
> Go ahead if it's for 5.15-rc, I don't have any selftests patches pending.
> 

Thanks Paolo. I will apply these for rc3.

thanks,
-- Shuah