linux-kernel - Re: [PATCH] msleep() with hrtimers

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]

Date:	Tue, 7 Aug 2007 21:14:01 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Roman Zippel <zippel@...ux-m68k.org>
Cc:	Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] msleep() with hrtimers

On Wed, 8 Aug 2007 05:47:02 +0200 (CEST) Roman Zippel <zippel@...ux-m68k.org> wrote:

> 
> 
> On Tue, 7 Aug 2007, Andrew Morton wrote:
> 
> > On Wed, 8 Aug 2007 01:16:49 +0200 (CEST)
> > Roman Zippel <zippel@...ux-m68k.org> wrote:
> > 
> > > Hi,
> > > 
> > > On Tue, 7 Aug 2007, Andrew Morton wrote:
> > > 
> > > > I'd be surprised if there was significant overhead - the maximum frequency
> > > > at which msleep() can be called is 1000Hz.  We'd need an awful lot of
> > > > overhead for that to cause problems, surely?
> > > > 
> > > > <thinks he's missing something again>
> > > 
> > > _Anybody_ has yet to answer what's wrong with adding a nanosleep() and 
> > > using that instead.
> > > 
> > 
> > You mean that the implementation could be simplified if msleep() were to
> > simply call do_nanosleep()?
> 
> The current msleep is fine and doesn't need any "fixing".
> Not all the world is i386, _please_ keep hrtimer usage where it's 
> required. Simple timer should be given preference unless the higher 
> resolution is really needed, which is not the case here.

Hang on.  Having msleep(1) sleep for 20 milliseconds is really awful
behaviour.  Possibly worse is the fact that with other configs, it will
delay for eight milliseconds.  Or two.  That's an order of magnitude of
unpredictability which can actually cause driver breakage.

Fixing that *bug* is a good thing.  I see no reason why we should "keep
hrtimer usage where it is required"?  The implementation details are hidden
from the caller.

It seems to be a sensible change to me and I fail to understand the
objection.

> so below is a nanosleep implementation based 
> on Jonathan's patch. This will user give a choice, so there is no need to 
> force all users to use hrtimer for a simple sleep.

But apart from needlessly fattening the kernel API, that leaves us in the
current situation where an unknown number of the msleep() callers actually
care that they are calling a function which by a huge margin fails to do
what they are asking it to do.  It will take a long time to hunt down all
the problematic callsites and migrate them to nanosleep().

-
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/