linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
@ 2015-03-25 23:44 John Stultz
  2015-03-25 23:44 ` [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat John Stultz
  2015-03-26 11:31 ` [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM Prarit Bhargava
  0 siblings, 2 replies; 20+ messages in thread
From: John Stultz @ 2015-03-25 23:44 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Shuah Khan, Prarit Bhargava, Thomas Gleixner,
	Richard Cochran

The set-timer-lat test fails when testing CLOCK_BOOTTIME_ALARM
or CLOCK_REALTIME_ALARM when the user isn't running as root or
with CAP_WAKE_ALARM.

So this patch improves the error checking so we report the
issue more clearly and continue rather then reporting a failure.

Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/set-timer-lat.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
index 3ea2eff..dbc9537c 100644
--- a/tools/testing/selftests/timers/set-timer-lat.c
+++ b/tools/testing/selftests/timers/set-timer-lat.c
@@ -139,6 +139,13 @@ int do_timer(int clock_id, int flags)
 
 	err = timer_create(clock_id, &se, &tm1);
 	if (err) {
+		if ((clock_id == CLOCK_REALTIME_ALARM)
+				|| (clock_id == CLOCK_BOOTTIME_ALARM)) {
+			printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
+					clockstring(clock_id),
+					flags ? "ABSTIME":"RELTIME");
+			return 0;
+		}
 		printf("%s - timer_create() failed\n", clockstring(clock_id));
 		return -1;
 	}
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
  2015-03-25 23:44 [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM John Stultz
@ 2015-03-25 23:44 ` John Stultz
  2015-03-26 11:32   ` Prarit Bhargava
  2015-03-26 11:31 ` [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM Prarit Bhargava
  1 sibling, 1 reply; 20+ messages in thread
From: John Stultz @ 2015-03-25 23:44 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Shuah Khan, Prarit Bhargava, Thomas Gleixner,
	Richard Cochran

For the default run_timers target, the timers tests takes the
majority of kselftests runtime.

So this patch reduces the default runtime for inconsistentcy-check
and set-timer-lat, which reduced the runtime almost in half.

Before:	11m48.629s
After:	6m47.723s

Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/inconsistency-check.c | 2 +-
 tools/testing/selftests/timers/set-timer-lat.c       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c
index 578e423a..caf1bc9 100644
--- a/tools/testing/selftests/timers/inconsistency-check.c
+++ b/tools/testing/selftests/timers/inconsistency-check.c
@@ -166,7 +166,7 @@ int main(int argc, char *argv[])
 	int clockid, opt;
 	int userclock = CLOCK_REALTIME;
 	int maxclocks = NR_CLOCKIDS;
-	int runtime = 30;
+	int runtime = 10;
 	struct timespec ts;
 
 	/* Process arguments */
diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
index dbc9537c..ba365d9 100644
--- a/tools/testing/selftests/timers/set-timer-lat.c
+++ b/tools/testing/selftests/timers/set-timer-lat.c
@@ -58,7 +58,7 @@ static inline int ksft_exit_fail(void)
 #define NSEC_PER_SEC 1000000000ULL
 #define UNRESONABLE_LATENCY 40000000 /* 40ms in nanosecs */
 
-#define TIMER_SECS 3
+#define TIMER_SECS 1
 int alarmcount;
 int clock_id;
 struct timespec start_time;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-25 23:44 [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM John Stultz
  2015-03-25 23:44 ` [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat John Stultz
@ 2015-03-26 11:31 ` Prarit Bhargava
  2015-03-26 16:29   ` John Stultz
  2015-03-31 15:55   ` Shuah Khan
  1 sibling, 2 replies; 20+ messages in thread
From: Prarit Bhargava @ 2015-03-26 11:31 UTC (permalink / raw)
  To: John Stultz; +Cc: lkml, Shuah Khan, Thomas Gleixner, Richard Cochran



On 03/25/2015 07:44 PM, John Stultz wrote:
> The set-timer-lat test fails when testing CLOCK_BOOTTIME_ALARM
> or CLOCK_REALTIME_ALARM when the user isn't running as root or
> with CAP_WAKE_ALARM.
> 
> So this patch improves the error checking so we report the
> issue more clearly and continue rather then reporting a failure.
> 
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  tools/testing/selftests/timers/set-timer-lat.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
> index 3ea2eff..dbc9537c 100644
> --- a/tools/testing/selftests/timers/set-timer-lat.c
> +++ b/tools/testing/selftests/timers/set-timer-lat.c
> @@ -139,6 +139,13 @@ int do_timer(int clock_id, int flags)
>  
>  	err = timer_create(clock_id, &se, &tm1);
>  	if (err) {
> +		if ((clock_id == CLOCK_REALTIME_ALARM)
> +				|| (clock_id == CLOCK_BOOTTIME_ALARM)) {

I dunno of there is actually a CodingStyle rule for this, but I've always seen
this written with the operator on the first line:

	if ((clock_id == CLOCK_REALTIME_ALARM) ||
            (clock_id == CLOCK_BOOTTIME_ALARM)) {

> +			printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
> +					clockstring(clock_id),
> +					flags ? "ABSTIME":"RELTIME");

Something to think about:  Do you want to write these tests to be more human
readable or machine readable?  In theory with awk I guess it doesn't matter too
much, however, it is something that we should think about moving forward.

P.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
  2015-03-25 23:44 ` [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat John Stultz
@ 2015-03-26 11:32   ` Prarit Bhargava
  2015-03-26 16:20     ` John Stultz
  0 siblings, 1 reply; 20+ messages in thread
From: Prarit Bhargava @ 2015-03-26 11:32 UTC (permalink / raw)
  To: John Stultz; +Cc: lkml, Shuah Khan, Thomas Gleixner, Richard Cochran



On 03/25/2015 07:44 PM, John Stultz wrote:
> For the default run_timers target, the timers tests takes the
> majority of kselftests runtime.
> 
> So this patch reduces the default runtime for inconsistentcy-check
> and set-timer-lat, which reduced the runtime almost in half.
> 
> Before:	11m48.629s
> After:	6m47.723s
> 
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  tools/testing/selftests/timers/inconsistency-check.c | 2 +-
>  tools/testing/selftests/timers/set-timer-lat.c       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c
> index 578e423a..caf1bc9 100644
> --- a/tools/testing/selftests/timers/inconsistency-check.c
> +++ b/tools/testing/selftests/timers/inconsistency-check.c
> @@ -166,7 +166,7 @@ int main(int argc, char *argv[])
>  	int clockid, opt;
>  	int userclock = CLOCK_REALTIME;
>  	int maxclocks = NR_CLOCKIDS;
> -	int runtime = 30;
> +	int runtime = 10;
>  	struct timespec ts;
>  

Oops ... left everyone off :)

What was the reason that this was originally 30?  Or was that overkill?

P.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
  2015-03-26 11:32   ` Prarit Bhargava
@ 2015-03-26 16:20     ` John Stultz
  2015-03-31 16:01       ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: John Stultz @ 2015-03-26 16:20 UTC (permalink / raw)
  To: Prarit Bhargava; +Cc: lkml, Shuah Khan, Thomas Gleixner, Richard Cochran

On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 03/25/2015 07:44 PM, John Stultz wrote:
>> For the default run_timers target, the timers tests takes the
>> majority of kselftests runtime.
>>
>> So this patch reduces the default runtime for inconsistentcy-check
>> and set-timer-lat, which reduced the runtime almost in half.
>>
>> Before:       11m48.629s
>> After:        6m47.723s
>>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Cc: Prarit Bhargava <prarit@redhat.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>>  tools/testing/selftests/timers/inconsistency-check.c | 2 +-
>>  tools/testing/selftests/timers/set-timer-lat.c       | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c
>> index 578e423a..caf1bc9 100644
>> --- a/tools/testing/selftests/timers/inconsistency-check.c
>> +++ b/tools/testing/selftests/timers/inconsistency-check.c
>> @@ -166,7 +166,7 @@ int main(int argc, char *argv[])
>>       int clockid, opt;
>>       int userclock = CLOCK_REALTIME;
>>       int maxclocks = NR_CLOCKIDS;
>> -     int runtime = 30;
>> +     int runtime = 10;
>>       struct timespec ts;
>>
>
> Oops ... left everyone off :)
>
> What was the reason that this was originally 30?  Or was that overkill?

So time inconsistencies (when they manifest, which ideally is never)
can be fairly rare events. In the past we've seen them due to cpu TSC
skew and drift, which requires enough scheduler noise to pop the
process around between cores enough to notice, and enough system
runtime for the TSCs to drift far enough apart.. Or we've had tiny
accumulation bugs in update_wall_time which requires the right phase
in the error accumulation to align with an irq. So the consistency
test has always been a long running test (originally I'd run it
overnight), and the 30sec interval here was added just so there was
some "long enough" interval that wasn't too painful for me to test
submitted patches with.  Now that more folks are using it (and they
likely care less), we can cut it down further to avoid making test
runs too onerous.

Now, a patch might badly break things and it would be immediately
obvious to the test that something is wrong, so a quick check isn't
worthless, but it just doesn't instill that much confidence from me.

I think as the kselftests grow, we'll have more "types" of test
targets to run (quick, long, stress, etc), and we can scale the time
in those tests accordingly. But the default should probably lean
towards the short side.

thanks
-john

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-26 11:31 ` [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM Prarit Bhargava
@ 2015-03-26 16:29   ` John Stultz
  2015-03-26 17:33     ` Tyler Baker
  2015-04-02 10:14     ` Prarit Bhargava
  2015-03-31 15:55   ` Shuah Khan
  1 sibling, 2 replies; 20+ messages in thread
From: John Stultz @ 2015-03-26 16:29 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: lkml, Shuah Khan, Thomas Gleixner, Richard Cochran, Tyler Baker

On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 03/25/2015 07:44 PM, John Stultz wrote:
>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>> +                                     clockstring(clock_id),
>> +                                     flags ? "ABSTIME":"RELTIME");
>
> Something to think about:  Do you want to write these tests to be more human
> readable or machine readable?  In theory with awk I guess it doesn't matter too
> much, however, it is something that we should think about moving forward.

So this came up at ELC in a few discussions. Right now there isn't any
established output format, but there's some nice and simple
infrastructure for counting pass/fails.

However, in talking to Tyler, I know he has started looking at how to
integrate the selftests into our automated infrastructure and was
interested in how we improve the output parsing for reports. So there
is interest in improving this, and I'm open to whatever changes might
be needed (adding extra arguments to the test to put them into "easy
parse" mode or whatever).

thanks
-john

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-26 16:29   ` John Stultz
@ 2015-03-26 17:33     ` Tyler Baker
  2015-04-02 10:18       ` Prarit Bhargava
  2015-04-02 10:14     ` Prarit Bhargava
  1 sibling, 1 reply; 20+ messages in thread
From: Tyler Baker @ 2015-03-26 17:33 UTC (permalink / raw)
  To: John Stultz
  Cc: Prarit Bhargava, lkml, Shuah Khan, Thomas Gleixner, Richard Cochran

On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>> +                                     clockstring(clock_id),
>>> +                                     flags ? "ABSTIME":"RELTIME");
>>
>> Something to think about:  Do you want to write these tests to be more human
>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>> much, however, it is something that we should think about moving forward.
>
> So this came up at ELC in a few discussions. Right now there isn't any
> established output format, but there's some nice and simple
> infrastructure for counting pass/fails.
>
> However, in talking to Tyler, I know he has started looking at how to
> integrate the selftests into our automated infrastructure and was
> interested in how we improve the output parsing for reports. So there
> is interest in improving this, and I'm open to whatever changes might
> be needed (adding extra arguments to the test to put them into "easy
> parse" mode or whatever).

Thanks for looping me in John. My interest in kselftest stems from my
involvement with kernelci.org, a communityservice focused on upstream
kernel validation across multiple architectures. In it's current form,
it is merely build and boot testing boards. However, we are at a point
where we'd like to start running some tests. The automation framework
(LAVA) used to execute these tests essentially uses a regular
expression to parse the test's standard output. This is advantageous
as a test can be written in any language, as long as it produces sane
uniform output.

Ideally, we would like to perform the kernel builds as we do today
along with building all the kseltests present in the tree, and
inserting them into a 'testing' ramdisk for deployment. Once we
successfully boot the platform, we execute all the kselftests, parse
standard out, and report the results. The benefit from this
implementation is that a developer writing a test does have to do
anything 'special' to get his/her test to run once it has been applied
to a upstream tree. I'll explain below some concerns I have about
accomplishing this.

Currently, we have had to write wrappers[1][2] for some kselftests to
be able parse the output. If we can choose/agree on a standard output
format all of this complexity goes away, and then we can dynamically
run kselftests. Integration of new tests will not be needed, as they
all produce output in standard way. I've taken a look at the wiki page
for standardizing output[3] and TAP looks like the good format IMO.

Also, for arch != x86 there are some barriers to overcome to get all
the kselftests cross compiling, which would be nice to have as well.

I realize this may be a good amount of work, so I'd like to help out.
Perhaps working John to convert his timer tests to use TAP output
would be a good starting point?

>
> thanks
> -john

[1] https://git.linaro.org/qa/test-definitions.git/blob/HEAD:/common/scripts/kselftest-runner.sh
[2] https://git.linaro.org/qa/test-definitions.git/blob/HEAD:/common/scripts/kselftest-mqueue.sh
[3] https://kselftest.wiki.kernel.org/standardize_the_test_output

Cheers,

Tyler

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-26 11:31 ` [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM Prarit Bhargava
  2015-03-26 16:29   ` John Stultz
@ 2015-03-31 15:55   ` Shuah Khan
  2015-04-02  3:42     ` John Stultz
  1 sibling, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2015-03-31 15:55 UTC (permalink / raw)
  To: John Stultz; +Cc: Prarit Bhargava, lkml, Thomas Gleixner, Richard Cochran

Hi John,

I am seeing checkpatch warnings on this patch. See below.

On 03/26/2015 05:31 AM, Prarit Bhargava wrote:
> 
> 
> On 03/25/2015 07:44 PM, John Stultz wrote:
>> The set-timer-lat test fails when testing CLOCK_BOOTTIME_ALARM
>> or CLOCK_REALTIME_ALARM when the user isn't running as root or
>> with CAP_WAKE_ALARM.
>>
>> So this patch improves the error checking so we report the
>> issue more clearly and continue rather then reporting a failure.
>>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Cc: Prarit Bhargava <prarit@redhat.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>

WARNING: Duplicate signature
#115:
Signed-off-by: John Stultz <john.stultz@linaro.org>

>> ---
>>  tools/testing/selftests/timers/set-timer-lat.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
>> index 3ea2eff..dbc9537c 100644
>> --- a/tools/testing/selftests/timers/set-timer-lat.c
>> +++ b/tools/testing/selftests/timers/set-timer-lat.c
>> @@ -139,6 +139,13 @@ int do_timer(int clock_id, int flags)
>>  
>>  	err = timer_create(clock_id, &se, &tm1);
>>  	if (err) {
>> +		if ((clock_id == CLOCK_REALTIME_ALARM)
>> +				|| (clock_id == CLOCK_BOOTTIME_ALARM)) {
> 
> I dunno of there is actually a CodingStyle rule for this, but I've always seen
> this written with the operator on the first line:

Yes it would be good to fix this one as well when you re-do the patch.


> 
> 	if ((clock_id == CLOCK_REALTIME_ALARM) ||
>             (clock_id == CLOCK_BOOTTIME_ALARM)) {
> 
>> +			printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>> +					clockstring(clock_id),
>> +					flags ? "ABSTIME":"RELTIME");
> 

WARNING: line over 80 characters
#130: FILE: tools/testing/selftests/timers/set-timer-lat.c:144:
+			printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
  2015-03-26 16:20     ` John Stultz
@ 2015-03-31 16:01       ` Shuah Khan
  2015-03-31 19:47         ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2015-03-31 16:01 UTC (permalink / raw)
  To: John Stultz, Prarit Bhargava; +Cc: lkml, Thomas Gleixner, Richard Cochran

On 03/26/2015 10:20 AM, John Stultz wrote:
> On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>> For the default run_timers target, the timers tests takes the
>>> majority of kselftests runtime.
>>>
>>> So this patch reduces the default runtime for inconsistentcy-check
>>> and set-timer-lat, which reduced the runtime almost in half.
>>>
>>> Before:       11m48.629s
>>> After:        6m47.723s
>>>
>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>> Cc: Prarit Bhargava <prarit@redhat.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Richard Cochran <richardcochran@gmail.com>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>

Same duplicate signature warning on this, no need to re-send.
I will fix it when I apply the patch.

-- Shuah
>>> ---
>>>  tools/testing/selftests/timers/inconsistency-check.c | 2 +-
>>>  tools/testing/selftests/timers/set-timer-lat.c       | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/timers/inconsistency-check.c b/tools/testing/selftests/timers/inconsistency-check.c
>>> index 578e423a..caf1bc9 100644
>>> --- a/tools/testing/selftests/timers/inconsistency-check.c
>>> +++ b/tools/testing/selftests/timers/inconsistency-check.c
>>> @@ -166,7 +166,7 @@ int main(int argc, char *argv[])
>>>       int clockid, opt;
>>>       int userclock = CLOCK_REALTIME;
>>>       int maxclocks = NR_CLOCKIDS;
>>> -     int runtime = 30;
>>> +     int runtime = 10;
>>>       struct timespec ts;
>>>
>>
>> Oops ... left everyone off :)
>>
>> What was the reason that this was originally 30?  Or was that overkill?
> 
> So time inconsistencies (when they manifest, which ideally is never)
> can be fairly rare events. In the past we've seen them due to cpu TSC
> skew and drift, which requires enough scheduler noise to pop the
> process around between cores enough to notice, and enough system
> runtime for the TSCs to drift far enough apart.. Or we've had tiny
> accumulation bugs in update_wall_time which requires the right phase
> in the error accumulation to align with an irq. So the consistency
> test has always been a long running test (originally I'd run it
> overnight), and the 30sec interval here was added just so there was
> some "long enough" interval that wasn't too painful for me to test
> submitted patches with.  Now that more folks are using it (and they
> likely care less), we can cut it down further to avoid making test
> runs too onerous.
> 
> Now, a patch might badly break things and it would be immediately
> obvious to the test that something is wrong, so a quick check isn't
> worthless, but it just doesn't instill that much confidence from me.
> 
> I think as the kselftests grow, we'll have more "types" of test
> targets to run (quick, long, stress, etc), and we can scale the time
> in those tests accordingly. But the default should probably lean
> towards the short side.
> 

Right. I am working on adding support for quick, long etc. The goal
for quick (default) mode is to complete the test runs in 15-20 minutes
to make it easier for developers make it part of the work-flow.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
  2015-03-31 16:01       ` Shuah Khan
@ 2015-03-31 19:47         ` Shuah Khan
  0 siblings, 0 replies; 20+ messages in thread
From: Shuah Khan @ 2015-03-31 19:47 UTC (permalink / raw)
  To: John Stultz, Prarit Bhargava
  Cc: lkml, Thomas Gleixner, Richard Cochran, Shuah Khan

On 03/31/2015 10:01 AM, Shuah Khan wrote:
> On 03/26/2015 10:20 AM, John Stultz wrote:
>> On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>> For the default run_timers target, the timers tests takes the
>>>> majority of kselftests runtime.
>>>>
>>>> So this patch reduces the default runtime for inconsistentcy-check
>>>> and set-timer-lat, which reduced the runtime almost in half.
>>>>
>>>> Before:       11m48.629s
>>>> After:        6m47.723s
>>>>
>>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>>> Cc: Prarit Bhargava <prarit@redhat.com>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Richard Cochran <richardcochran@gmail.com>
>>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> Same duplicate signature warning on this, no need to re-send.
> I will fix it when I apply the patch.
> 

Thanks for doing this. I saw similar results on my test system.
Applied to linux-kselftest next for 4.1

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-31 15:55   ` Shuah Khan
@ 2015-04-02  3:42     ` John Stultz
  0 siblings, 0 replies; 20+ messages in thread
From: John Stultz @ 2015-04-02  3:42 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Prarit Bhargava, lkml, Thomas Gleixner, Richard Cochran

On Tue, Mar 31, 2015 at 8:55 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> Hi John,
>
> I am seeing checkpatch warnings on this patch. See below.

Sorry!

> On 03/26/2015 05:31 AM, Prarit Bhargava wrote:
>>
>>
>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>> The set-timer-lat test fails when testing CLOCK_BOOTTIME_ALARM
>>> or CLOCK_REALTIME_ALARM when the user isn't running as root or
>>> with CAP_WAKE_ALARM.
>>>
>>> So this patch improves the error checking so we report the
>>> issue more clearly and continue rather then reporting a failure.
>>>
>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>> Cc: Prarit Bhargava <prarit@redhat.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Richard Cochran <richardcochran@gmail.com>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> WARNING: Duplicate signature
> #115:
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Fixed.

>>> ---
>>>  tools/testing/selftests/timers/set-timer-lat.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
>>> index 3ea2eff..dbc9537c 100644
>>> --- a/tools/testing/selftests/timers/set-timer-lat.c
>>> +++ b/tools/testing/selftests/timers/set-timer-lat.c
>>> @@ -139,6 +139,13 @@ int do_timer(int clock_id, int flags)
>>>
>>>      err = timer_create(clock_id, &se, &tm1);
>>>      if (err) {
>>> +            if ((clock_id == CLOCK_REALTIME_ALARM)
>>> +                            || (clock_id == CLOCK_BOOTTIME_ALARM)) {
>>
>> I dunno of there is actually a CodingStyle rule for this, but I've always seen
>> this written with the operator on the first line:
>
> Yes it would be good to fix this one as well when you re-do the patch.

Fixed.

>>
>>       if ((clock_id == CLOCK_REALTIME_ALARM) ||
>>             (clock_id == CLOCK_BOOTTIME_ALARM)) {
>>
>>> +                    printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>> +                                    clockstring(clock_id),
>>> +                                    flags ? "ABSTIME":"RELTIME");
>>
>
> WARNING: line over 80 characters
> #130: FILE: tools/testing/selftests/timers/set-timer-lat.c:144:
> +                       printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",

This I probably will leave, as the alternative is breaking the text
string across lines, which checkpatch will also gripe about.

thanks
-john

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-26 16:29   ` John Stultz
  2015-03-26 17:33     ` Tyler Baker
@ 2015-04-02 10:14     ` Prarit Bhargava
  1 sibling, 0 replies; 20+ messages in thread
From: Prarit Bhargava @ 2015-04-02 10:14 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Shuah Khan, Thomas Gleixner, Richard Cochran, Tyler Baker



On 03/26/2015 12:29 PM, John Stultz wrote:
> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>> +                                     clockstring(clock_id),
>>> +                                     flags ? "ABSTIME":"RELTIME");
>>
>> Something to think about:  Do you want to write these tests to be more human
>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>> much, however, it is something that we should think about moving forward.
> 
> So this came up at ELC in a few discussions. Right now there isn't any\

Sorry, I'm not familiar with ELC?  What is that Acronym for?
> established output format, but there's some nice and simple
> infrastructure for counting pass/fails.

Okay that's great.

> 
> However, in talking to Tyler, I know he has started looking at how to
> integrate the selftests into our automated infrastructure and was
> interested in how we improve the output parsing for reports. So there
> is interest in improving this, and I'm open to whatever changes might
> be needed (adding extra arguments to the test to put them into "easy
> parse" mode or whatever).

Thanks John.

P.

> 
> thanks
> -john
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-03-26 17:33     ` Tyler Baker
@ 2015-04-02 10:18       ` Prarit Bhargava
  2015-04-02 13:43         ` Shuah Khan
  2015-04-02 18:02         ` John Stultz
  0 siblings, 2 replies; 20+ messages in thread
From: Prarit Bhargava @ 2015-04-02 10:18 UTC (permalink / raw)
  To: Tyler Baker
  Cc: John Stultz, lkml, Shuah Khan, Thomas Gleixner, Richard Cochran



On 03/26/2015 01:33 PM, Tyler Baker wrote:
> On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>>> +                                     clockstring(clock_id),
>>>> +                                     flags ? "ABSTIME":"RELTIME");
>>>
>>> Something to think about:  Do you want to write these tests to be more human
>>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>>> much, however, it is something that we should think about moving forward.
>>
>> So this came up at ELC in a few discussions. Right now there isn't any
>> established output format, but there's some nice and simple
>> infrastructure for counting pass/fails.
>>
>> However, in talking to Tyler, I know he has started looking at how to
>> integrate the selftests into our automated infrastructure and was
>> interested in how we improve the output parsing for reports. So there
>> is interest in improving this, and I'm open to whatever changes might
>> be needed (adding extra arguments to the test to put them into "easy
>> parse" mode or whatever).
> 
> Thanks for looping me in John. My interest in kselftest stems from my
> involvement with kernelci.org, a communityservice focused on upstream
> kernel validation across multiple architectures. In it's current form,
> it is merely build and boot testing boards. However, we are at a point
> where we'd like to start running some tests. The automation framework
> (LAVA) used to execute these tests essentially uses a regular
> expression to parse the test's standard output. This is advantageous
> as a test can be written in any language, as long as it produces sane
> uniform output.
> 
> Ideally, we would like to perform the kernel builds as we do today
> along with building all the kseltests present in the tree, and
> inserting them into a 'testing' ramdisk for deployment. Once we
> successfully boot the platform, we execute all the kselftests, parse
> standard out, and report the results. The benefit from this
> implementation is that a developer writing a test does have to do
> anything 'special' to get his/her test to run once it has been applied
> to a upstream tree. I'll explain below some concerns I have about
> accomplishing this.
> 
> Currently, we have had to write wrappers[1][2] for some kselftests to
> be able parse the output. If we can choose/agree on a standard output
> format all of this complexity goes away, and then we can dynamically
> run kselftests. Integration of new tests will not be needed, as they
> all produce output in standard way. I've taken a look at the wiki page
> for standardizing output[3] and TAP looks like the good format IMO.
> 
> Also, for arch != x86 there are some barriers to overcome to get all
> the kselftests cross compiling, which would be nice to have as well.
> 
> I realize this may be a good amount of work, so I'd like to help out.
> Perhaps working John to convert his timer tests to use TAP output
> would be a good starting point?

John, I could probably do that for you.  I'm always willing to give it a shot.

> 
>>
>> thanks
>> -john
> 
> [1] https://git.linaro.org/qa/test-definitions.git/blob/HEAD:/common/scripts/kselftest-runner.sh
> [2] https://git.linaro.org/qa/test-definitions.git/blob/HEAD:/common/scripts/kselftest-mqueue.sh
> [3] https://kselftest.wiki.kernel.org/standardize_the_test_output

I'll go off and look at this and wait for the current patchset(s) to make it
into Linus' tree before posting or suggesting patches.

P.

> 
> Cheers,
> 
> Tyler
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 10:18       ` Prarit Bhargava
@ 2015-04-02 13:43         ` Shuah Khan
  2015-04-02 17:17           ` Tyler Baker
  2015-04-02 18:02         ` John Stultz
  1 sibling, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2015-04-02 13:43 UTC (permalink / raw)
  To: Prarit Bhargava, Tyler Baker
  Cc: John Stultz, lkml, Thomas Gleixner, Richard Cochran

On 04/02/2015 04:18 AM, Prarit Bhargava wrote:
> 
> 
> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>> On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
>>> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>>>> +                                     clockstring(clock_id),
>>>>> +                                     flags ? "ABSTIME":"RELTIME");
>>>>
>>>> Something to think about:  Do you want to write these tests to be more human
>>>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>>>> much, however, it is something that we should think about moving forward.
>>>
>>> So this came up at ELC in a few discussions. Right now there isn't any
>>> established output format, but there's some nice and simple
>>> infrastructure for counting pass/fails.
>>>
>>> However, in talking to Tyler, I know he has started looking at how to
>>> integrate the selftests into our automated infrastructure and was
>>> interested in how we improve the output parsing for reports. So there
>>> is interest in improving this, and I'm open to whatever changes might
>>> be needed (adding extra arguments to the test to put them into "easy
>>> parse" mode or whatever).
>>
>> Thanks for looping me in John. My interest in kselftest stems from my
>> involvement with kernelci.org, a communityservice focused on upstream
>> kernel validation across multiple architectures. In it's current form,
>> it is merely build and boot testing boards. However, we are at a point
>> where we'd like to start running some tests. The automation framework
>> (LAVA) used to execute these tests essentially uses a regular
>> expression to parse the test's standard output. This is advantageous
>> as a test can be written in any language, as long as it produces sane
>> uniform output.
>>
>> Ideally, we would like to perform the kernel builds as we do today
>> along with building all the kseltests present in the tree, and
>> inserting them into a 'testing' ramdisk for deployment. Once we
>> successfully boot the platform, we execute all the kselftests, parse
>> standard out, and report the results. The benefit from this
>> implementation is that a developer writing a test does have to do
>> anything 'special' to get his/her test to run once it has been applied
>> to a upstream tree. I'll explain below some concerns I have about
>> accomplishing this.
>>
>> Currently, we have had to write wrappers[1][2] for some kselftests to
>> be able parse the output. If we can choose/agree on a standard output
>> format all of this complexity goes away, and then we can dynamically
>> run kselftests. Integration of new tests will not be needed, as they
>> all produce output in standard way. I've taken a look at the wiki page
>> for standardizing output[3] and TAP looks like the good format IMO.
>>
>> Also, for arch != x86 there are some barriers to overcome to get all
>> the kselftests cross compiling, which would be nice to have as well.
>>
>> I realize this may be a good amount of work, so I'd like to help out.
>> Perhaps working John to convert his timer tests to use TAP output
>> would be a good starting point?
> 
> John, I could probably do that for you.  I'm always willing to give it a shot.
> 

Improving reporting and output is a good idea, as long as the
reporting framework doesn't add external dependencies and makes
it difficult to build and run tests from the kernel git tree
environment. Being able to run tests on a development system
is the primary objective for these tests.

As long as the TAP doesn't require additional libraries and
tools to be installed on the development systems, I will be
happy with the improvements to reporting and output.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 13:43         ` Shuah Khan
@ 2015-04-02 17:17           ` Tyler Baker
  2015-04-02 17:48             ` Shuah Khan
  0 siblings, 1 reply; 20+ messages in thread
From: Tyler Baker @ 2015-04-02 17:17 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Prarit Bhargava, John Stultz, lkml, Thomas Gleixner, Richard Cochran

On 2 April 2015 at 06:43, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 04/02/2015 04:18 AM, Prarit Bhargava wrote:
>>
>>
>> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>>> On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
>>>> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>>>>> +                                     clockstring(clock_id),
>>>>>> +                                     flags ? "ABSTIME":"RELTIME");
>>>>>
>>>>> Something to think about:  Do you want to write these tests to be more human
>>>>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>>>>> much, however, it is something that we should think about moving forward.
>>>>
>>>> So this came up at ELC in a few discussions. Right now there isn't any
>>>> established output format, but there's some nice and simple
>>>> infrastructure for counting pass/fails.
>>>>
>>>> However, in talking to Tyler, I know he has started looking at how to
>>>> integrate the selftests into our automated infrastructure and was
>>>> interested in how we improve the output parsing for reports. So there
>>>> is interest in improving this, and I'm open to whatever changes might
>>>> be needed (adding extra arguments to the test to put them into "easy
>>>> parse" mode or whatever).
>>>
>>> Thanks for looping me in John. My interest in kselftest stems from my
>>> involvement with kernelci.org, a communityservice focused on upstream
>>> kernel validation across multiple architectures. In it's current form,
>>> it is merely build and boot testing boards. However, we are at a point
>>> where we'd like to start running some tests. The automation framework
>>> (LAVA) used to execute these tests essentially uses a regular
>>> expression to parse the test's standard output. This is advantageous
>>> as a test can be written in any language, as long as it produces sane
>>> uniform output.
>>>
>>> Ideally, we would like to perform the kernel builds as we do today
>>> along with building all the kseltests present in the tree, and
>>> inserting them into a 'testing' ramdisk for deployment. Once we
>>> successfully boot the platform, we execute all the kselftests, parse
>>> standard out, and report the results. The benefit from this
>>> implementation is that a developer writing a test does have to do
>>> anything 'special' to get his/her test to run once it has been applied
>>> to a upstream tree. I'll explain below some concerns I have about
>>> accomplishing this.
>>>
>>> Currently, we have had to write wrappers[1][2] for some kselftests to
>>> be able parse the output. If we can choose/agree on a standard output
>>> format all of this complexity goes away, and then we can dynamically
>>> run kselftests. Integration of new tests will not be needed, as they
>>> all produce output in standard way. I've taken a look at the wiki page
>>> for standardizing output[3] and TAP looks like the good format IMO.
>>>
>>> Also, for arch != x86 there are some barriers to overcome to get all
>>> the kselftests cross compiling, which would be nice to have as well.
>>>
>>> I realize this may be a good amount of work, so I'd like to help out.
>>> Perhaps working John to convert his timer tests to use TAP output
>>> would be a good starting point?
>>
>> John, I could probably do that for you.  I'm always willing to give it a shot.
>>
>
> Improving reporting and output is a good idea, as long as the
> reporting framework doesn't add external dependencies and makes
> it difficult to build and run tests from the kernel git tree
> environment. Being able to run tests on a development system
> is the primary objective for these tests.
>
> As long as the TAP doesn't require additional libraries and
> tools to be installed on the development systems, I will be
> happy with the improvements to reporting and output.

This is a very valid concern IMO. There is a C library for TAP
providers[0] but I agree we want to avoid external libraries as
dependencies. Perhaps, it would be simple enough to write a selftest
harness that provides a basic interface for displaying test results,
messages, and warning/errors. This harness would ensure all the
logging was done in a TAP compliant manner. Food for thought :)

>
> thanks,
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978

Cheers,

Tyler

[0] http://www.eyrie.org/~eagle/software/c-tap-harness/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 17:17           ` Tyler Baker
@ 2015-04-02 17:48             ` Shuah Khan
  2015-04-02 18:58               ` Tyler Baker
  0 siblings, 1 reply; 20+ messages in thread
From: Shuah Khan @ 2015-04-02 17:48 UTC (permalink / raw)
  To: Tyler Baker
  Cc: Prarit Bhargava, John Stultz, lkml, Thomas Gleixner,
	Richard Cochran, Shuah Khan

On 04/02/2015 11:17 AM, Tyler Baker wrote:
> On 2 April 2015 at 06:43, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 04/02/2015 04:18 AM, Prarit Bhargava wrote:
>>>
>>>
>>> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>>>> On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
>>>>> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>>>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>>>>>> +                                     clockstring(clock_id),
>>>>>>> +                                     flags ? "ABSTIME":"RELTIME");
>>>>>>
>>>>>> Something to think about:  Do you want to write these tests to be more human
>>>>>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>>>>>> much, however, it is something that we should think about moving forward.
>>>>>
>>>>> So this came up at ELC in a few discussions. Right now there isn't any
>>>>> established output format, but there's some nice and simple
>>>>> infrastructure for counting pass/fails.
>>>>>
>>>>> However, in talking to Tyler, I know he has started looking at how to
>>>>> integrate the selftests into our automated infrastructure and was
>>>>> interested in how we improve the output parsing for reports. So there
>>>>> is interest in improving this, and I'm open to whatever changes might
>>>>> be needed (adding extra arguments to the test to put them into "easy
>>>>> parse" mode or whatever).
>>>>
>>>> Thanks for looping me in John. My interest in kselftest stems from my
>>>> involvement with kernelci.org, a communityservice focused on upstream
>>>> kernel validation across multiple architectures. In it's current form,
>>>> it is merely build and boot testing boards. However, we are at a point
>>>> where we'd like to start running some tests. The automation framework
>>>> (LAVA) used to execute these tests essentially uses a regular
>>>> expression to parse the test's standard output. This is advantageous
>>>> as a test can be written in any language, as long as it produces sane
>>>> uniform output.
>>>>
>>>> Ideally, we would like to perform the kernel builds as we do today
>>>> along with building all the kseltests present in the tree, and
>>>> inserting them into a 'testing' ramdisk for deployment. Once we
>>>> successfully boot the platform, we execute all the kselftests, parse
>>>> standard out, and report the results. The benefit from this
>>>> implementation is that a developer writing a test does have to do
>>>> anything 'special' to get his/her test to run once it has been applied
>>>> to a upstream tree. I'll explain below some concerns I have about
>>>> accomplishing this.
>>>>
>>>> Currently, we have had to write wrappers[1][2] for some kselftests to
>>>> be able parse the output. If we can choose/agree on a standard output
>>>> format all of this complexity goes away, and then we can dynamically
>>>> run kselftests. Integration of new tests will not be needed, as they
>>>> all produce output in standard way. I've taken a look at the wiki page
>>>> for standardizing output[3] and TAP looks like the good format IMO.
>>>>
>>>> Also, for arch != x86 there are some barriers to overcome to get all
>>>> the kselftests cross compiling, which would be nice to have as well.
>>>>
>>>> I realize this may be a good amount of work, so I'd like to help out.
>>>> Perhaps working John to convert his timer tests to use TAP output
>>>> would be a good starting point?
>>>
>>> John, I could probably do that for you.  I'm always willing to give it a shot.
>>>
>>
>> Improving reporting and output is a good idea, as long as the
>> reporting framework doesn't add external dependencies and makes
>> it difficult to build and run tests from the kernel git tree
>> environment. Being able to run tests on a development system
>> is the primary objective for these tests.
>>
>> As long as the TAP doesn't require additional libraries and
>> tools to be installed on the development systems, I will be
>> happy with the improvements to reporting and output.
> 
> This is a very valid concern IMO. There is a C library for TAP
> providers[0] but I agree we want to avoid external libraries as
> dependencies. Perhaps, it would be simple enough to write a selftest
> harness that provides a basic interface for displaying test results,
> messages, and warning/errors. This harness would ensure all the
> logging was done in a TAP compliant manner. Food for thought :)
> 

The harness has to cover tests in written in C as well as shell
scripts. I added a simple framework in kselftest.h as a step
towards harness. However, it needs more work. I would welcome
any patches that would provide a harness that meets the needs
of external selftest use-cases such as yours.

Is writing a simple TAP compliant harness something you can take
on?? Looking for volunteers :)

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 10:18       ` Prarit Bhargava
  2015-04-02 13:43         ` Shuah Khan
@ 2015-04-02 18:02         ` John Stultz
  2015-04-07 14:20           ` Prarit Bhargava
  2015-04-08  4:03           ` Michael Ellerman
  1 sibling, 2 replies; 20+ messages in thread
From: John Stultz @ 2015-04-02 18:02 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Tyler Baker, lkml, Shuah Khan, Thomas Gleixner, Richard Cochran

On Thu, Apr 2, 2015 at 3:18 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>> I realize this may be a good amount of work, so I'd like to help out.
>> Perhaps working John to convert his timer tests to use TAP output
>> would be a good starting point?
>
> John, I could probably do that for you.  I'm always willing to give it a shot.

I took a quick look into it, since I'm definitely interested in
improving output formatting, but man, TAP is a fairly ugly output
format if you ask me.

It only has binary "ok" or "not ok" (why not "fail", or something else
that's exclusively grep-able, I don't know). So I'm not sure if cases
where functionality is unsupported should be a pass or fail.

Most problematically: It seems to want enumeration in the test output
(so test 2 needs to print: "ok 2 Test complete") which means either
there needs to be a wrapper that does the TAP output knowing which
test of N its currently running, or the test number needs to be
submitted as an runtime argument to the test, and the test then has to
add it to its output line.

Anyway, if we do want to go with that format,  I suspect it should be
something we add to the kselftest pass/fail hooks, rather then to the
individual tests. Then its just a matter of prefixing normal test
output with #'s so they can be ignored by the parser.

thanks
-john

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 17:48             ` Shuah Khan
@ 2015-04-02 18:58               ` Tyler Baker
  0 siblings, 0 replies; 20+ messages in thread
From: Tyler Baker @ 2015-04-02 18:58 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Prarit Bhargava, John Stultz, lkml, Thomas Gleixner, Richard Cochran

On 2 April 2015 at 10:48, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 04/02/2015 11:17 AM, Tyler Baker wrote:
>> On 2 April 2015 at 06:43, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 04/02/2015 04:18 AM, Prarit Bhargava wrote:
>>>>
>>>>
>>>> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>>>>> On 26 March 2015 at 09:29, John Stultz <john.stultz@linaro.org> wrote:
>>>>>> On Thu, Mar 26, 2015 at 4:31 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>>>>>> On 03/25/2015 07:44 PM, John Stultz wrote:
>>>>>>>> +                     printf("%-22s %s missing CAP_WAKE_ALARM?    : [UNSUPPORTED]\n",
>>>>>>>> +                                     clockstring(clock_id),
>>>>>>>> +                                     flags ? "ABSTIME":"RELTIME");
>>>>>>>
>>>>>>> Something to think about:  Do you want to write these tests to be more human
>>>>>>> readable or machine readable?  In theory with awk I guess it doesn't matter too
>>>>>>> much, however, it is something that we should think about moving forward.
>>>>>>
>>>>>> So this came up at ELC in a few discussions. Right now there isn't any
>>>>>> established output format, but there's some nice and simple
>>>>>> infrastructure for counting pass/fails.
>>>>>>
>>>>>> However, in talking to Tyler, I know he has started looking at how to
>>>>>> integrate the selftests into our automated infrastructure and was
>>>>>> interested in how we improve the output parsing for reports. So there
>>>>>> is interest in improving this, and I'm open to whatever changes might
>>>>>> be needed (adding extra arguments to the test to put them into "easy
>>>>>> parse" mode or whatever).
>>>>>
>>>>> Thanks for looping me in John. My interest in kselftest stems from my
>>>>> involvement with kernelci.org, a communityservice focused on upstream
>>>>> kernel validation across multiple architectures. In it's current form,
>>>>> it is merely build and boot testing boards. However, we are at a point
>>>>> where we'd like to start running some tests. The automation framework
>>>>> (LAVA) used to execute these tests essentially uses a regular
>>>>> expression to parse the test's standard output. This is advantageous
>>>>> as a test can be written in any language, as long as it produces sane
>>>>> uniform output.
>>>>>
>>>>> Ideally, we would like to perform the kernel builds as we do today
>>>>> along with building all the kseltests present in the tree, and
>>>>> inserting them into a 'testing' ramdisk for deployment. Once we
>>>>> successfully boot the platform, we execute all the kselftests, parse
>>>>> standard out, and report the results. The benefit from this
>>>>> implementation is that a developer writing a test does have to do
>>>>> anything 'special' to get his/her test to run once it has been applied
>>>>> to a upstream tree. I'll explain below some concerns I have about
>>>>> accomplishing this.
>>>>>
>>>>> Currently, we have had to write wrappers[1][2] for some kselftests to
>>>>> be able parse the output. If we can choose/agree on a standard output
>>>>> format all of this complexity goes away, and then we can dynamically
>>>>> run kselftests. Integration of new tests will not be needed, as they
>>>>> all produce output in standard way. I've taken a look at the wiki page
>>>>> for standardizing output[3] and TAP looks like the good format IMO.
>>>>>
>>>>> Also, for arch != x86 there are some barriers to overcome to get all
>>>>> the kselftests cross compiling, which would be nice to have as well.
>>>>>
>>>>> I realize this may be a good amount of work, so I'd like to help out.
>>>>> Perhaps working John to convert his timer tests to use TAP output
>>>>> would be a good starting point?
>>>>
>>>> John, I could probably do that for you.  I'm always willing to give it a shot.
>>>>
>>>
>>> Improving reporting and output is a good idea, as long as the
>>> reporting framework doesn't add external dependencies and makes
>>> it difficult to build and run tests from the kernel git tree
>>> environment. Being able to run tests on a development system
>>> is the primary objective for these tests.
>>>
>>> As long as the TAP doesn't require additional libraries and
>>> tools to be installed on the development systems, I will be
>>> happy with the improvements to reporting and output.
>>
>> This is a very valid concern IMO. There is a C library for TAP
>> providers[0] but I agree we want to avoid external libraries as
>> dependencies. Perhaps, it would be simple enough to write a selftest
>> harness that provides a basic interface for displaying test results,
>> messages, and warning/errors. This harness would ensure all the
>> logging was done in a TAP compliant manner. Food for thought :)
>>
>
> The harness has to cover tests in written in C as well as shell
> scripts. I added a simple framework in kselftest.h as a step
> towards harness. However, it needs more work. I would welcome
> any patches that would provide a harness that meets the needs
> of external selftest use-cases such as yours.

Ok, I'll have a closer look at kselftest.h.

>
> Is writing a simple TAP compliant harness something you can take
> on?? Looking for volunteers :)

I'll volunteer to pick up these work items :) I should be able to
start on this work at some point next week.

John brought up some good points about the TAP format, and I'm
personally not yet convinced it is the best format. That being said,
I'll work to keep the changes to the harness generic so that we can
easily switch test output formats if we decide there is something
better than TAP in the future.

>
> thanks,
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978

Thanks,

Tyler

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 18:02         ` John Stultz
@ 2015-04-07 14:20           ` Prarit Bhargava
  2015-04-08  4:03           ` Michael Ellerman
  1 sibling, 0 replies; 20+ messages in thread
From: Prarit Bhargava @ 2015-04-07 14:20 UTC (permalink / raw)
  To: John Stultz
  Cc: Tyler Baker, lkml, Shuah Khan, Thomas Gleixner, Richard Cochran



On 04/02/2015 02:02 PM, John Stultz wrote:
> On Thu, Apr 2, 2015 at 3:18 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>> On 03/26/2015 01:33 PM, Tyler Baker wrote:
>>> I realize this may be a good amount of work, so I'd like to help out.
>>> Perhaps working John to convert his timer tests to use TAP output
>>> would be a good starting point?
>>
>> John, I could probably do that for you.  I'm always willing to give it a shot.
> 
> I took a quick look into it, since I'm definitely interested in
> improving output formatting, but man, TAP is a fairly ugly output
> format if you ask me.
> 
> It only has binary "ok" or "not ok" (why not "fail", or something else
> that's exclusively grep-able, I don't know). So I'm not sure if cases
> where functionality is unsupported should be a pass or fail.
> 
> Most problematically: It seems to want enumeration in the test output
> (so test 2 needs to print: "ok 2 Test complete") which means either
> there needs to be a wrapper that does the TAP output knowing which
> test of N its currently running, or the test number needs to be
> submitted as an runtime argument to the test, and the test then has to
> add it to its output line.
> 
> Anyway, if we do want to go with that format,  I suspect it should be
> something we add to the kselftest pass/fail hooks, rather then to the
> individual tests. Then its just a matter of prefixing normal test
> output with #'s so they can be ignored by the parser.

I've been looking into this and while TAP is useful, I find myself with a few
questions:

1.  I want the output to be both human and machine readable.  I find that "not
okay" vs "okay" is confusing at best for humans to read.  I suppose that is a
minor issue, but it is a real issue when laymen are asked to execute a test.

2.  I think we can get around the TAP numbering, however, maybe a better
approach is to limit tests to doing one, and only one, test?  ie) something like
rtctest.c would have be be broken up into individual tests.

3.  TAP appears to have a BSD-style license?  Not knowing enough about cross
licensing issues like this, is that going to be a problem?

4.  I think John is moving in the right direction by suggesting that we move to
putting this into kselftest.h as hooks, rather than modifying each test.
Another idea is to include a script that allows the running of multiple tests
from the top level tests directory.

Let me muck around with the timer tests and see what I can come up with.  I'm
trying to keep in mind that we're going to be asking people to potentially
modify their tests, as well as what we're going to be telling people to do when
they add new tests, while I make modifications.

P.


> 
> thanks
> -john
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM
  2015-04-02 18:02         ` John Stultz
  2015-04-07 14:20           ` Prarit Bhargava
@ 2015-04-08  4:03           ` Michael Ellerman
  1 sibling, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2015-04-08  4:03 UTC (permalink / raw)
  To: John Stultz
  Cc: Prarit Bhargava, Tyler Baker, lkml, Shuah Khan, Thomas Gleixner,
	Richard Cochran, Robert Collins

On Thu, 2015-04-02 at 11:02 -0700, John Stultz wrote:
> On Thu, Apr 2, 2015 at 3:18 AM, Prarit Bhargava <prarit@redhat.com> wrote:
> > On 03/26/2015 01:33 PM, Tyler Baker wrote:
> >> I realize this may be a good amount of work, so I'd like to help out.
> >> Perhaps working John to convert his timer tests to use TAP output
> >> would be a good starting point?
> >
> > John, I could probably do that for you.  I'm always willing to give it a shot.
> 
> I took a quick look into it, since I'm definitely interested in
> improving output formatting, but man, TAP is a fairly ugly output
> format if you ask me.
> 
> It only has binary "ok" or "not ok" (why not "fail", or something else
> that's exclusively grep-able, I don't know). So I'm not sure if cases
> where functionality is unsupported should be a pass or fail.
> 
> Most problematically: It seems to want enumeration in the test output
> (so test 2 needs to print: "ok 2 Test complete") which means either
> there needs to be a wrapper that does the TAP output knowing which
> test of N its currently running, or the test number needs to be
> submitted as an runtime argument to the test, and the test then has to
> add it to its output line.

Yeah TAP is horrible, for the reasons you describe.

I *think* in practice most tools will handle just "ok" / "not ok" without the
tests being numbered, but I don't know that for sure.


For the powerpc tests I'm using "subunit v1" [1], which is basically just:
  - "^success: <test name>"
  - "^failure: <test name>"

See: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/powerpc/subunit.h

So I can do eg:

$ cd tools/testing/selftests/powerpc
$ make run_tests 2>&1 | subunit-1to2 | subunit-stats --no-passthrough
Total tests:      35
Passed tests:     31
Failed tests:      0
Skipped tests:     4
Seen tags: git_version:v4.0-rc7-0-gf22e6e8


But unfortunately TAP has a lot more traction with tools.

It wouldn't be too hard to convert the subunit stream into TAP I think, but for
some reason no one seems to have written that. Maybe we should, I just haven't
had time to do it.

cheers


[1]: https://github.com/testing-cabal/subunit/blob/master/README#L343




^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-04-08  4:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-25 23:44 [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM John Stultz
2015-03-25 23:44 ` [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat John Stultz
2015-03-26 11:32   ` Prarit Bhargava
2015-03-26 16:20     ` John Stultz
2015-03-31 16:01       ` Shuah Khan
2015-03-31 19:47         ` Shuah Khan
2015-03-26 11:31 ` [PATCH 1/2] kselftests: timers: Make set-timer-lat fail more gracefully for !CAP_WAKE_ALARM Prarit Bhargava
2015-03-26 16:29   ` John Stultz
2015-03-26 17:33     ` Tyler Baker
2015-04-02 10:18       ` Prarit Bhargava
2015-04-02 13:43         ` Shuah Khan
2015-04-02 17:17           ` Tyler Baker
2015-04-02 17:48             ` Shuah Khan
2015-04-02 18:58               ` Tyler Baker
2015-04-02 18:02         ` John Stultz
2015-04-07 14:20           ` Prarit Bhargava
2015-04-08  4:03           ` Michael Ellerman
2015-04-02 10:14     ` Prarit Bhargava
2015-03-31 15:55   ` Shuah Khan
2015-04-02  3:42     ` John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).