Messages in this thread | | | Date | Mon, 12 Mar 2007 12:38:29 +0100 | From | Ingo Molnar <> | Subject | Re: [patch] change futex_wait() to hrtimers |
| |
* 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.
> > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |