Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-18 Thread Michael Ellerman
On Wed, 2015-03-18 at 09:55 -0600, Shuah Khan wrote:
> On 03/15/2015 08:48 PM, Michael Ellerman wrote:
> > On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
> >> My only thoughts:
> >> 1) Would it be better if threadtest can be made to have better
> >> defaults for kselftest so you don't need that extra logic?
> > 
> > That would help. But with the patch I just sent I think it's no bother, it's
> > only a little extra logic and it's only in the timers Makefile.
> 
> Let's go with a threadtest patch with better defaults. It will emit
> scripts logic as well. Awesome. I just saw John's new patch in my
> Inbox. Thanks.

Fine by me.

> >> 2) While I get that TEST_FILES is likely going to be used to copy the
> >> destructive tests over, It feels a little like its being bundled in
> >> with something like data files that tests might need, which seems sort
> >> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> >> make more sense?
> > 
> > That doesn't really bother me. You're right that TEST_FILES is originally
> > intended for data files etc. but I don't think it's a big hack to use it for
> > other tests that shouldn't be run by default. Still if it bothers you I'm 
> > happy
> > to add a separate variable for it, they are cheap :)
> 
> Could you please make the change from TEST_FILES to TEST_PROGS_EXTENDED
> which is definitely better than overloading TEST_FILES?

I don't see the point, but I don't care that much.

> I think this would probably only change Add Install target patch. You
> can send me patch v5 with that change and I will override the next with
> your new one. Thanks for doing this.

You shouldn't rebase your next branch, people may have already merged it (like
me). You should apply it as an additional patch on top.

Patch sent.

cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-18 Thread Shuah Khan
On 03/15/2015 08:48 PM, Michael Ellerman wrote:
> On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
>> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan  wrote:
>>> Change the timers Makefile to make use of shared run and install
>>> logic in lib.mk. Destructive tests are installed. Regular tests
>>> are emited to run_kselftest script to match the run_tests behavior.
>>>
>>> Signed-off-by: Shuah Khan 
>>> ---
>>>  tools/testing/selftests/timers/Makefile | 20 +++-
>>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/timers/Makefile 
>>> b/tools/testing/selftests/timers/Makefile
>>> index 9da3498..61e7284 100644
>>> --- a/tools/testing/selftests/timers/Makefile
>>> +++ b/tools/testing/selftests/timers/Makefile
>>> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check 
>>> nsleep-lat raw_skew \
>>> alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>>> leap-a-day leapcrash set-tai set-2038
>>>
>>> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
>>> +   inconsistency-check raw_skew
>>> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
>>> +   skew_consistency clocksource-switch leap-a-day leapcrash \
>>> +   set-tai set-2038
>>> +
>>> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: 
>>> threadtest [FAIL]"
>>> +
>>> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
>>> +
>>
>> So my make-foo isn't very strong, but no objections from me.
>>
>> My only thoughts:
>> 1) Would it be better if threadtest can be made to have better
>> defaults for kselftest so you don't need that extra logic?
> 
> That would help. But with the patch I just sent I think it's no bother, it's
> only a little extra logic and it's only in the timers Makefile.

Let's go with a threadtest patch with better defaults. It will emit
scripts logic as well. Awesome. I just saw John's new patch in my
Inbox. Thanks.

> 
>> 2) While I get that TEST_FILES is likely going to be used to copy the
>> destructive tests over, It feels a little like its being bundled in
>> with something like data files that tests might need, which seems sort
>> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
>> make more sense?
> 
> That doesn't really bother me. You're right that TEST_FILES is originally
> intended for data files etc. but I don't think it's a big hack to use it for
> other tests that shouldn't be run by default. Still if it bothers you I'm 
> happy
> to add a separate variable for it, they are cheap :)
> 

Michael,

Could you please make the change from TEST_FILES to TEST_PROGS_EXTENDED
which is definitely better than overloading TEST_FILES?

I think this would probably only change Add Install target patch. You
can send me patch v5 with that change and I will override the next with
your new one. Thanks for doing this.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-18 Thread Shuah Khan
On 03/13/2015 09:14 PM, John Stultz wrote:
> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan  wrote:
>> Change the timers Makefile to make use of shared run and install
>> logic in lib.mk. Destructive tests are installed. Regular tests
>> are emited to run_kselftest script to match the run_tests behavior.
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  tools/testing/selftests/timers/Makefile | 20 +++-
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/Makefile 
>> b/tools/testing/selftests/timers/Makefile
>> index 9da3498..61e7284 100644
>> --- a/tools/testing/selftests/timers/Makefile
>> +++ b/tools/testing/selftests/timers/Makefile
>> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check 
>> nsleep-lat raw_skew \
>> alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>> leap-a-day leapcrash set-tai set-2038
>>
>> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
>> +   inconsistency-check raw_skew
>> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
>> +   skew_consistency clocksource-switch leap-a-day leapcrash \
>> +   set-tai set-2038
>> +
>> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: 
>> threadtest [FAIL]"
>> +
>> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
>> +
> 
> So my make-foo isn't very strong, but no objections from me.
> 
> My only thoughts:
> 1) Would it be better if threadtest can be made to have better
> defaults for kselftest so you don't need that extra logic?

Yes. If you can change the threadset to run with -t 30 -n 8
in default case, that would make the logic simpler. Can you
send me a patch to do this?


> 2) While I get that TEST_FILES is likely going to be used to copy the
> destructive tests over, It feels a little like its being bundled in
> with something like data files that tests might need, which seems sort
> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> make more sense?

I agree with you on this. TEST_FILES usage is an overload.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-15 Thread Michael Ellerman
On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan  wrote:
> > Change the timers Makefile to make use of shared run and install
> > logic in lib.mk. Destructive tests are installed. Regular tests
> > are emited to run_kselftest script to match the run_tests behavior.
> >
> > Signed-off-by: Shuah Khan 
> > ---
> >  tools/testing/selftests/timers/Makefile | 20 +++-
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/timers/Makefile 
> > b/tools/testing/selftests/timers/Makefile
> > index 9da3498..61e7284 100644
> > --- a/tools/testing/selftests/timers/Makefile
> > +++ b/tools/testing/selftests/timers/Makefile
> > @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check 
> > nsleep-lat raw_skew \
> > alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> > leap-a-day leapcrash set-tai set-2038
> >
> > +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
> > +   inconsistency-check raw_skew
> > +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
> > +   skew_consistency clocksource-switch leap-a-day leapcrash \
> > +   set-tai set-2038
> > +
> > +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: 
> > threadtest [FAIL]"
> > +
> > +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
> > +
> 
> So my make-foo isn't very strong, but no objections from me.
> 
> My only thoughts:
> 1) Would it be better if threadtest can be made to have better
> defaults for kselftest so you don't need that extra logic?

That would help. But with the patch I just sent I think it's no bother, it's
only a little extra logic and it's only in the timers Makefile.

> 2) While I get that TEST_FILES is likely going to be used to copy the
> destructive tests over, It feels a little like its being bundled in
> with something like data files that tests might need, which seems sort
> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> make more sense?

That doesn't really bother me. You're right that TEST_FILES is originally
intended for data files etc. but I don't think it's a big hack to use it for
other tests that shouldn't be run by default. Still if it bothers you I'm happy
to add a separate variable for it, they are cheap :)

cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-15 Thread Michael Ellerman
On Sun, 2015-03-15 at 19:42 +1100, Michael Ellerman wrote:
> 
> On 14 March 2015 09:57:51 GMT+11:00, Shuah Khan  
> wrote:
> >Change the timers Makefile to make use of shared run and install
> >logic in lib.mk. Destructive tests are installed. Regular tests
> >are emited to run_kselftest script to match the run_tests behavior.
> >
> >Signed-off-by: Shuah Khan 
> >---
> > tools/testing/selftests/timers/Makefile | 20 +++-
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> >diff --git a/tools/testing/selftests/timers/Makefile
> >b/tools/testing/selftests/timers/Makefile
> >index 9da3498..61e7284 100644
> >--- a/tools/testing/selftests/timers/Makefile
> >+++ b/tools/testing/selftests/timers/Makefile
> >@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check
> >nsleep-lat raw_skew \
> > alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> > leap-a-day leapcrash set-tai set-2038
> > 
> >+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat
> >mqueue-lat \
> >+inconsistency-check raw_skew
> >+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew
> >\
> >+skew_consistency clocksource-switch leap-a-day leapcrash \
> >+set-tai set-2038
> >+
> >+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests:
> >threadtest [FAIL]"
> 
> You shouldn't need this separate variable. As long as you override RUN_TESTS 
> after you include lib.mk you can include the default value, eg:
> 
> override RUN_TESTS := $(RUN_TESTS) ./threadtest -t 30 -n 8 || echo 
> "selftests: threadtest [FAIL]"
> 
> I'll test that in the morning and send a proper patch.

How's this look?

Contents of install/timers (same as with your patch):

$ ls install/timers/
alarmtimer-suspend*  clocksource-switch*   leap-a-day*  mqueue-lat*  
nsleep-lat*raw_skew*  set-tai*skew_consistency*  valid-adjtimex*
change_skew* inconsistency-check*  leapcrash*   nanosleep*   
posix_timers*  set-2038*  set-timer-lat*  threadtest*

And in run_kselftest.sh:

echo ; echo Running tests in timers
echo 
cd timers
(./posix_timers && echo "selftests: posix_timers [PASS]") || echo "selftests: 
posix_timers [FAIL]"
(./nanosleep && echo "selftests: nanosleep [PASS]") || echo "selftests: 
nanosleep [FAIL]"
(./nsleep-lat && echo "selftests: nsleep-lat [PASS]") || echo "selftests: 
nsleep-lat [FAIL]"
(./set-timer-lat && echo "selftests: set-timer-lat [PASS]") || echo "selftests: 
set-timer-lat [FAIL]"
(./mqueue-lat && echo "selftests: mqueue-lat [PASS]") || echo "selftests: 
mqueue-lat [FAIL]"
(./inconsistency-check && echo "selftests: inconsistency-check [PASS]") || echo 
"selftests: inconsistency-check [FAIL]"
(./raw_skew && echo "selftests: raw_skew [PASS]") || echo "selftests: raw_skew 
[FAIL]"
./threadtest -t 30 -n 8 || echo selftests: threadtest [FAIL]
cd $ROOT


cheers



>From 5ea5ea0e5adee5317978965acc4594dce648da63 Mon Sep 17 00:00:00 2001
From: Michael Ellerman 
Date: Mon, 16 Mar 2015 13:38:06 +1100
Subject: [PATCH] selftests/timers: Use shared logic to run and install tests

Change the timers Makefile to make use of shared run and install logic
in lib.mk. Destructive tests are installed. Regular tests are emited to
run_kselftest script to match the run_tests behavior.

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/timers/Makefile | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/timers/Makefile 
b/tools/testing/selftests/timers/Makefile
index 9da3498987c8..8ba056ee8b48 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -1,25 +1,26 @@
-CC = $(CROSS_COMPILE)gcc
 BUILD_FLAGS = -DKTEST
 CFLAGS += -O3 -Wl,-no-as-needed -Wall $(BUILD_FLAGS)
 LDFLAGS += -lrt -lpthread
-bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
-   set-timer-lat threadtest mqueue-lat valid-adjtimex \
-   alarmtimer-suspend change_skew skew_consistency clocksource-switch \
-   leap-a-day leapcrash set-tai set-2038
-
-all: ${bins}
 
 # these are all "safe" tests that don't modify
 # system time or require escalated privledges
-run_tests: all
-   ./posix_timers
-   ./nanosleep
-   ./nsleep-lat
-   ./set-timer-lat
-   ./mqueue-lat
-   ./inconsistency-check
-   ./raw_skew
-   ./threadtest -t 30 -n 8
+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
+   inconsistency-check raw_skew
+
+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
+   skew_consistency clocksource-switch leap-a-day leapcrash \
+   set-tai set-2038
+
+bins = $(TEST_PROGS) $(TEST_FILES)
+
+all: ${bins}
+
+include ../lib.mk
+
+THREADTEST := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
+
+override RUN_TESTS := $(RUN_TESTS) $(THREADTEST)
+override EMIT_TESTS 

Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-15 Thread Michael Ellerman


On 14 March 2015 09:57:51 GMT+11:00, Shuah Khan  wrote:
>Change the timers Makefile to make use of shared run and install
>logic in lib.mk. Destructive tests are installed. Regular tests
>are emited to run_kselftest script to match the run_tests behavior.
>
>Signed-off-by: Shuah Khan 
>---
> tools/testing/selftests/timers/Makefile | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
>diff --git a/tools/testing/selftests/timers/Makefile
>b/tools/testing/selftests/timers/Makefile
>index 9da3498..61e7284 100644
>--- a/tools/testing/selftests/timers/Makefile
>+++ b/tools/testing/selftests/timers/Makefile
>@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check
>nsleep-lat raw_skew \
>   alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>   leap-a-day leapcrash set-tai set-2038
> 
>+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat
>mqueue-lat \
>+  inconsistency-check raw_skew
>+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew
>\
>+  skew_consistency clocksource-switch leap-a-day leapcrash \
>+  set-tai set-2038
>+
>+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests:
>threadtest [FAIL]"

You shouldn't need this separate variable. As long as you override RUN_TESTS 
after you include lib.mk you can include the default value, eg:

override RUN_TESTS := $(RUN_TESTS) ./threadtest -t 30 -n 8 || echo "selftests: 
threadtest [FAIL]"

I'll test that in the morning and send a proper patch.

cheers
-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-13 Thread John Stultz
On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan  wrote:
> Change the timers Makefile to make use of shared run and install
> logic in lib.mk. Destructive tests are installed. Regular tests
> are emited to run_kselftest script to match the run_tests behavior.
>
> Signed-off-by: Shuah Khan 
> ---
>  tools/testing/selftests/timers/Makefile | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/timers/Makefile 
> b/tools/testing/selftests/timers/Makefile
> index 9da3498..61e7284 100644
> --- a/tools/testing/selftests/timers/Makefile
> +++ b/tools/testing/selftests/timers/Makefile
> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check 
> nsleep-lat raw_skew \
> alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> leap-a-day leapcrash set-tai set-2038
>
> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
> +   inconsistency-check raw_skew
> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
> +   skew_consistency clocksource-switch leap-a-day leapcrash \
> +   set-tai set-2038
> +
> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: 
> threadtest [FAIL]"
> +
> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
> +

So my make-foo isn't very strong, but no objections from me.

My only thoughts:
1) Would it be better if threadtest can be made to have better
defaults for kselftest so you don't need that extra logic?
2) While I get that TEST_FILES is likely going to be used to copy the
destructive tests over, It feels a little like its being bundled in
with something like data files that tests might need, which seems sort
of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
make more sense?

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/2] selftests/timers: change to use shared logic to run and install tests

2015-03-13 Thread Shuah Khan
Change the timers Makefile to make use of shared run and install
logic in lib.mk. Destructive tests are installed. Regular tests
are emited to run_kselftest script to match the run_tests behavior.

Signed-off-by: Shuah Khan 
---
 tools/testing/selftests/timers/Makefile | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/timers/Makefile 
b/tools/testing/selftests/timers/Makefile
index 9da3498..61e7284 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat 
raw_skew \
alarmtimer-suspend change_skew skew_consistency clocksource-switch \
leap-a-day leapcrash set-tai set-2038
 
+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
+   inconsistency-check raw_skew
+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
+   skew_consistency clocksource-switch leap-a-day leapcrash \
+   set-tai set-2038
+
+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest 
[FAIL]"
+
+EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
+
 all: ${bins}
 
 # these are all "safe" tests that don't modify
 # system time or require escalated privledges
-run_tests: all
-   ./posix_timers
-   ./nanosleep
-   ./nsleep-lat
-   ./set-timer-lat
-   ./mqueue-lat
-   ./inconsistency-check
-   ./raw_skew
-   ./threadtest -t 30 -n 8
+include ../lib.mk
 
 # these tests require escalated privledges
 # and may modify the system time or trigger
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/