Re: [patch] change futex_wait() to hrtimers

[Nick Piggin <npiggin@suse.de>]

On Mon, Mar 12, 2007 at 01:21:03PM +0100, Ingo Molnar wrote:
> 
> * Nick Piggin <npiggin@suse.de> wrote:
> 
> > > > > the issue is this: your fix reduces the effects of the bug but 
> > > > > it is still fundamentally incomplete because of the use of 
> > > > > timer_list. So
> > > > 
> > > > But using schedule_timeout is not a bug. Userspace timeouts are 
> > > > always defined to be "at least".
> > > 
> > > but what you are adding isnt a plain schedule_timeout(), it is a 
> > > restart block handling loop. And for those restart blocks that 
> > > relate to timeouts, we only use hrtimers. I am not making this up to 
> > > annoy you: take a look at all the current restart block handlers - 
> > > they are hrtimer based, for exactly this reason.
> > 
> > So why do you say it is fundamentally incomplete?
> 
> because i misread your last patch :-) I thought it still has a window 
> for inaccuracy, but you are right: it should be at most 1 jiffy 
> inaccurate, no matter how many times we restart.

OK, no problem.

> still ... the hrtimers patch has been submitted to lkml before yours, 
> and has been tested extensively, so why go the extra side-jump 
> prolonging the jiffies sleep method? The LTP failure has been there 
> since the inception of the futex code i suspect. Going this way also 
> enables the addressing of a more pressing need: the elimination of 
> glibc's forced use of relative futex timeouts.

I guess my arguments are that my patch fixes a bug, which gives it a
higher priority (being a userspace API bug, perhaps even 2.6.21); and
that it will want to be backported while the hrtimer patch will not, so
including the hrtimer patch first means 2 different patches to fix the
same bug.

I'm not trying to make life harder for the hrtimer patch. I will even
volunteer to forward port it on top of the restart fix, if that is an
issue.