Re: [patch] change futex_wait() to hrtimers

[Ingo Molnar]

* Nick Piggin <npiggin@xxxxxxx> 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.

> > instead of trying to fix the bug the wrong way, please try to fix it 
> > the right way, ontop of an already existing and tested patch, ok? 
> > That also enables the other neat stuff Thomas talked about.
> 
> Well that's nice, but I have a bugfix here which probably needs to get 
> backported to stable kernels and distro kernels.

yes but your patch already exists for them which they can pick up.

really, this is a common Linux principle: fix it completely and fix it 
the right way. You are applying it yourself on a daily basis when having 
the maintainer hat on =B-)

> It should be just as easy to rebase the hrtimer patch on top of my 
> fix. Considering that you've had it for a year, I don't think it needs 
> to be added right before my fix.

your latest patch looks quite kludgy, exactly due to the issues that 
were mentioned.

> > hm. I'm wondering how this wasnt noticed sooner - this futex_wait 
> > behavior has been there for like forever.
> 
> People ignore LTP test failures, and programs probably try to avoid 
> exercising the nuances of the unix signal API, I guess.

then there's no rush and lets do this the right way, ok?

	Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/