Re: [PATCH] doc: add note on usleep_range range

Nicholas Mc Guire
2017-01-07T19:42:28Z

On Tue, Dec 27, 2016 at 10:56:26PM +0100, Pavel Machek wrote:
> On Tue 2016-12-13 04:58:43, Nicholas Mc Guire wrote:
> > useleep_range() with a delta of 0 makes no sense and only prevents the
> > timer subsystem from optimizing interrupts. As any user of usleep_range()
> > is in non-atomic context the timer jitter is in the range of 10s of
> > microseconds anyway.
> >
> > This adds a note making it clear that a range of 0 is a bad idea.
> >
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > ---
> >
> > as of 4.9.0 there are about 20 cases of usleep_ranges() that have
> > min==max and none of them really look like they are necessary, so
> > it does seem like a relatively common misunderstanding worth
> > noting in the documentation.
> >
> > Patch is against 4.9.0 (localversion-next is 20161212)
> >
> > Documentation/timers/timers-howto.txt | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/timers/timers-howto.txt b/Documentation/timers/timers-howto.txt
> > index 038f8c7..b5cdf82 100644
> > --- a/Documentation/timers/timers-howto.txt
> > +++ b/Documentation/timers/timers-howto.txt
> > @@ -93,6 +93,13 @@ NON-ATOMIC CONTEXT:
> > tolerances here are very situation specific, thus it
> > is left to the caller to determine a reasonable range.
> >
> > + A range of 0, that is usleep_range(100,100) or the
> > + like, do not make sense as this code is in a
> > + non-atomic section and a system can not be expected
> > + to have jitter 0. For any non-RT code any delta
>
> Would it be possible to fix english here?

Aggreed that is crappy language - my bad - will fix - thanks!

>
> "to have zero jitter" at least. I believe it is "does not".
>
> I don't see how atomic vs. non-atomic context makes difference. There
> are sources of jitter that affect atomic context...

The relevance is that while there is jitter in atomic context it can
be quite small (depending on your hardware and the specifics of system
config) but in non-atomic context the jitter is so large that it
makes no relevant difference if you give usleep_range slack of a few
microseconds.

>
> > + less than 50 microseconds probably is only preventing
> > + timer subsystem optimization but providing no benefit.
>
> And I don't trust you here. _If_ it prevents timer optimalization,
> _then_ it provides benefit, at least in the average case.
>
here is the data:

System: Intel Core i7 CPU 920 @ 2.67GHz Ocotocore
OS: Debian 8.1 (but thats quite irrelevant)
Kernel: 4.10-rc2 (localversion-next next-20170106)
config: x86_64_defconfig (Voluntary | Preempt)

Test-setup - poped this into akernel module and just
brute force load/unload it in a loop - not very elegant
but it does the job.

static int __init usleep_test_init(void)
{
ktime_t now,last;
unsigned long min,max;
min = 200;
max = 250;
last = ktime_get();
usleep_range(min, max);
now = ktime_get();
printk("%llu\n", ktime_to_ns(now)-ktime_to_ns(last));
return 0;
}

Results:

usleep_range() 5000 samples - idle system
100,100 200,200 190,200
Min. :188481 Min. :201917 Min. :197793
1st Qu.:207062 1st Qu.:207057 1st Qu.:207051
Median :207139 Median :207133 Median :207133
Mean :207254 Mean :207233 Mean :207244
3rd Qu.:207341 erd Qu.:207262 3rd Qu.:207610
Max. :225340 Max. :214222 Max. :214885

100,200 to 200,200 is maybe relevant impact for
some systems with respect to the outliers, but
mean and median are almost the same, for
190,200 to 200,200 there is statistically no
significant difference with respect to performance
Note that the timestamp before and after also has
jitter - so only part of the jitter can be attributed
to usleep_range() it self. But idle system optimization
is not that interesting for most systems.

On a loaded box:
Load here means that 8 find / | grep bla loops were started
(not pinned to any particular core) and then the
usleep_range() test ran 5000 times.

Same setup as above but this time we differenciae between
PREEMTI and PREEMPT_VOLUNTARY

CONFIG_PREEMPT_VOLUNTARY=y
usleep_range() 5000 samples - load ~ 8
100,200 190,200 200,200
Min. : 107812 Min. : 203307 Min. : 203432
1st Qu.: 558221 1st Qu.: 557490 1st Qu.: 510356
Median :1123425 Median : 1121939 Median : 1123316
Mean :1103718 Mean : 1100965 Mean : 1100542
3rd Qu.:1541986 3rd Qu.: 1531478 3rd Qu.: 1517414
Max. :8979183 Max. :13765789 Max. :12476136

CONFIG_PREEMPT=y
usleep_range() 5000 samples - load ~ 8
100,200 190,200 200,200
Min. : 115321 Min. : 203963 Min. : 203864
1st Qu.: 510296 1st Qu.: 451479 1st Qu.: 548131
Median : 1148660 Median : 1062576 Median : 1145228
Mean : 1193449 Mean : 1079379 Mean : 1154728
3rd Qu.: 1601552 3rd Qu.: 1378622 3rd Qu.: 1570742
Max. :12936192 Max. :12346313 Max. :13858732

So for a loaded system it simply makes no relevant difference
if you grant the subsystem 10 microseconds range or not. In
fact if one wanted 200 us and would allow fow 200,250 it would
be quite hard to notice the difference.

usleep_range(200,200) vs usleep_range(200,250)
Min. : 203864 Min. : 214003
1st Qu.: 548131 1st Qu.: 520436
Median : 1145228 Median : 1138698
Mean : 1154728 Mean : 1201871
3rd Qu.: 1570742 3rd Qu.: 1581952
Max. :13858732 Max. :12491198

I would call the difference insignificant - ploted as curves you
can hardly tell the distribution appart. As soon as you are looking
at more than a single tasks to optimize the difference would
probably completely disapear.

thx!
hofrat