Messages in this thread | | | From | Jani Nikula <> | Subject | Re: [PATCH] doc: add note on usleep_range range | Date | Tue, 13 Dec 2016 11:10:50 +0200 |
| |
On Tue, 13 Dec 2016, Nicholas Mc Guire <hofrat@osadl.org> 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.
So I don't really have anything to do with the timer subsystem, I'm just their "consumer", so take this with a grain of salt.
Documentation is good, but I don't think this will be enough.
I think the only thing that will work is to detect and complain about things like this automatically. Some ideas:
* WARN_ON(min == max) or WARN_ON_ONCE(min == max) in usleep_range() might be drastic, but it would get the job done eventually.
* If you want to avoid the runtime overhead (and complaints about the backtraces), you could wrap usleep_range() in a macro that does BUILD_BUG_ON(min == max) if the parameters are build time constants (they usually are). But you'd have to fix all the problem cases first.
* You could try (to persuade Julia or Dan) to come up with a cocci/smatch check for usleep_range() calls where min == max, so we could get bug reports for this. This probably works on expressions, so this would catch also cases where the parameters aren't built time constants.
BR, Jani.
> > 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 > + less than 50 microseconds probably is only preventing > + timer subsystem optimization but providing no benefit. > + > SLEEPING FOR LARGER MSECS ( 10ms+ ) > * Use msleep or possibly msleep_interruptible
-- Jani Nikula, Intel Open Source Technology Center
| |