Re: [PATCH 1/4] timer: Added usleep[_range] timer

[Patrick Pannuto <ppannuto@codeaurora.org>]

> This is different from the patch I merged and I'm not seeing any
> explanation for the change.
> 
> The implementation of usleep() looks odd.  The longer we sleep, the
> greater the possible inaccuracy.  A code comment which explains the
> thinking and which warns people about the implications is needed.

Yes it is different; the explanation was in the cover message. I should
probably include a copy of the explanation in the commit message as
well? It was becoming a very long commit message...

// FROM COVER MESSAGE:
   This iteration is similar, with the notable difference that now
   usleep has a "built-in slack" of 200%. This is analogous to msleep,
   which has a built-in slack of 0.4% (since it relies on legacy timers,
   which have a built-in slack of 0.4%). 200% slack is significantly
   greater than 0.4%, but the scale of usleep is also significantly
   different than that of msleep, and I believe 200% to be a sane
   default.

   It is my opinion that this interface will most often mirror what
   developers actually intend - indeed some people who have begun
   trying to use the API raised this point -, however, I would like
   some input as it is possibly confusing that the API will "double
   your sleep" by default.

   The usleep_range API is still included, since it provides an
   interface to override the "default slack" of 200% by providing
   an explicit range, or to allow callers to specify an even larger
   slack if possible.

The problem that was raised by a few people trying to use usleep here
was that the API as written was very awkward -- there was never really
a good reason to use "usleep" as it was written. The intention was
to make usleep a usable / sensible API; the obvious alternative I see
is to drop the usleep function entirely and only provide usleep_range - 
which would probably fit well in your request for callers to think
about what they are doing, if providing a somewhat awkward API.

The complaint was something to the effect of:

   "Well, I understand that I should probably give a range, but I have
   no idea what a good range would be. I really just want it to sleep
   for a little bit, but I probably shouldn't trigger an extra interrupt.
   Given the limitations, what's the point of even having a usleep call
   at all?"


Thoughts?

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum