linux-kernel - Re: [PATCH 1/4] timer: Added usleep[

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C509772.1070407@codeaurora.org>
Date:	Wed, 28 Jul 2010 13:47:46 -0700
From:	Patrick Pannuto <ppannuto@...eaurora.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	linux-kernel@...r.kernel.org, apw@...onical.com, corbet@....net,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Akinobu Mita <akinobu.mita@...il.com>
Subject: Re: [PATCH 1/4] timer: Added usleep[_range] timer

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