Re: [PATCH 2/2] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
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 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 Cc: Prarit Bhargava Cc: Thomas Gleixner Cc: Richard Cochran Signed-off-by: John Stultz Signed-off-by: John Stultz > > 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) 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
On 03/26/2015 10:20 AM, John Stultz wrote: > On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava 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 >>> Cc: Prarit Bhargava >>> Cc: Thomas Gleixner >>> Cc: Richard Cochran >>> Signed-off-by: John Stultz >>> Signed-off-by: John Stultz 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) 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
On 03/26/2015 10:20 AM, John Stultz wrote: On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava pra...@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 shua...@osg.samsung.com Cc: Prarit Bhargava pra...@redhat.com Cc: Thomas Gleixner t...@linutronix.de Cc: Richard Cochran richardcoch...@gmail.com Signed-off-by: John Stultz john.stu...@linaro.org Signed-off-by: John Stultz john.stu...@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) 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
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 pra...@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 shua...@osg.samsung.com Cc: Prarit Bhargava pra...@redhat.com Cc: Thomas Gleixner t...@linutronix.de Cc: Richard Cochran richardcoch...@gmail.com Signed-off-by: John Stultz john.stu...@linaro.org Signed-off-by: John Stultz john.stu...@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) 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava 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 >> Cc: Prarit Bhargava >> Cc: Thomas Gleixner >> Cc: Richard Cochran >> Signed-off-by: John Stultz >> Signed-off-by: John Stultz >> --- >> 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 -- 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
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 > Cc: Prarit Bhargava > Cc: Thomas Gleixner > Cc: Richard Cochran > Signed-off-by: John Stultz > Signed-off-by: John Stultz > --- > 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. -- 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
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 shua...@osg.samsung.com Cc: Prarit Bhargava pra...@redhat.com Cc: Thomas Gleixner t...@linutronix.de Cc: Richard Cochran richardcoch...@gmail.com Signed-off-by: John Stultz john.stu...@linaro.org Signed-off-by: John Stultz john.stu...@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. -- 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] kselftests: timers: Reduce default runtime on inconsistency-check and set-timer-lat
On Thu, Mar 26, 2015 at 4:32 AM, Prarit Bhargava pra...@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 shua...@osg.samsung.com Cc: Prarit Bhargava pra...@redhat.com Cc: Thomas Gleixner t...@linutronix.de Cc: Richard Cochran richardcoch...@gmail.com Signed-off-by: John Stultz john.stu...@linaro.org Signed-off-by: John Stultz john.stu...@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 -- 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/
