Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755507Ab1DOOHX (ORCPT ); Fri, 15 Apr 2011 10:07:23 -0400 Received: from mga11.intel.com ([192.55.52.93]:1507 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751425Ab1DOOHV (ORCPT ); Fri, 15 Apr 2011 10:07:21 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.64,219,1301900400"; d="scan'208";a="910080099" Message-ID: <4DA85114.3010501@linux.intel.com> Date: Fri, 15 Apr 2011 07:07:16 -0700 From: Darren Hart User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8 MIME-Version: 1.0 To: Thomas Gleixner CC: Linux Kernel Mailing List , Eric Dumazet , Peter Zijlstra , Ingo Molnar , John Kacur , stable@kernel.org Subject: Re: [PATCH V5] futex: set FLAGS_HAS_TIMEOUT during futex_wait restart setup References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2946 Lines: 81 On 04/15/2011 01:33 AM, Thomas Gleixner wrote: > On Thu, 14 Apr 2011, Darren Hart wrote: > >> The FLAGS_HAS_TIMEOUT flag was not getting set, causing the restart_block to >> restart futex_wait() without a timeout after a signal. >> >> Commit b41277dc7a18ee332d in 2.6.38 introduced the regression by accidentally >> removing the the FLAGS_HAS_TIMEOUT assignment from futex_wait() during the setup >> of the restart block. Restore the originaly behavior. >> >> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=32922 >> >> V2: Added references to commit message. >> V3: Set flag during restart block instead of do_futex() >> V4: Correct stupid order of assignment mistake pointed out by Eric >> V5: Correct subject to match implementation, correct stable submission >> >> Signed-off-by: Darren Hart >> Signed-off-by: Eric Dumazet >> Reported-by: Tim Smith >> Reported-by: Torsten Hilbrich >> Cc: Thomas Gleixner >> Cc: Peter Zijlstra >> Cc: Ingo Molnar >> Cc: John Kacur >> Cc: stable@kernel.org >> --- >> kernel/futex.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/kernel/futex.c b/kernel/futex.c >> index bda4157..abd5324 100644 >> --- a/kernel/futex.c >> +++ b/kernel/futex.c >> @@ -1886,7 +1886,7 @@ retry: >> restart->futex.val = val; >> restart->futex.time = abs_time->tv64; >> restart->futex.bitset = bitset; >> - restart->futex.flags = flags; >> + restart->futex.flags = flags | FLAGS_HAS_TIMEOUT; > > We only get here when a timeout is pending. So why don't we just do > the obvious: > > --- linux-2.6.orig/kernel/futex.c > +++ linux-2.6/kernel/futex.c > @@ -1902,16 +1902,13 @@ out: > static long futex_wait_restart(struct restart_block *restart) > { > u32 __user *uaddr = restart->futex.uaddr; > - ktime_t t, *tp = NULL; > + ktime_t t; > > - if (restart->futex.flags & FLAGS_HAS_TIMEOUT) { > - t.tv64 = restart->futex.time; > - tp = &t; > - } > + t.tv64 = restart->futex.time; > restart->fn = do_no_restart_syscall; > > return (long)futex_wait(uaddr, restart->futex.flags, > - restart->futex.val, tp, restart->futex.bitset); > + restart->futex.val, &t, restart->futex.bitset); > } I believe I asked you the same question when I was adding the FLAGS_HAS_TIMEOUT. :-) The problem is distinguishing between no timeout and an expired timer. The above always passes a non-null address for abs_time to futex_wait(), which will then always schedule a timer. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel -- 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/