From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751415AbcFWHUt (ORCPT ); Thu, 23 Jun 2016 03:20:49 -0400 Received: from www.linutronix.de ([62.245.132.108]:46432 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751188AbcFWHUs (ORCPT ); Thu, 23 Jun 2016 03:20:48 -0400 Date: Thu, 23 Jun 2016 09:18:42 +0200 (CEST) From: Thomas Gleixner To: Darren Hart cc: Matthieu CASTET , linux-kernel@vger.kernel.org, Michael Kerrisk , Darren Hart , Peter Zijlstra , Davidlohr Bueso , Eric Dumazet Subject: Re: futex: Allow FUTEX_CLOCK_REALTIME with FUTEX_WAIT op In-Reply-To: <20160623044828.GA3379@f23x64.localdomain> Message-ID: References: <20160620162652.2c4619c3@perruche.parrot.biz> <20160623044828.GA3379@f23x64.localdomain> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 22 Jun 2016, Darren Hart wrote: > However, I don't think the patch below is correct. The existing logic > determines the type of timeout based on the futex_op when it should instead > determine the type of timeout based on the FUTEX_CLOCK_REALTIME flag. No. > My reading of the man page is that FUTEX_WAIT_BITSET abides by the timeout > interpretation defined by the FUTEX_CLOCK_REALTIME attribute, so > SYSCALL_DEFINE6 was misbehaving for FUTEX_WAIT|FUTEX_CLOCK_REALTIME (where the > timeout should have been treated as absolute) as well as for > FUTEX_WAIT_BITSET|FUTEX_CLOCK_MONOTONIC (where the timeout should have been > treated as relative). > > Consider the following: > > diff --git a/kernel/futex.c b/kernel/futex.c > index 33664f7..fa2af29 100644 > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -3230,7 +3230,7 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val, > return -EINVAL; > > t = timespec_to_ktime(ts); > - if (cmd == FUTEX_WAIT) > + if (!(cmd & FUTEX_CLOCK_REALTIME)) > t = ktime_add_safe(ktime_get(), t); That breaks LOCK_PI, REQUEUE_PI and FUTEX_WAIT_BITSET > The concern for me is whether the code is incorrect, or if the man page is > incorrect. Does existing userspace code expect the FUTEX_WAIT_BITSET op to > always use an absolute timeout, regardless of the CLOCK used? FUTEX_WAIT_BITSET, LOCK_PI and REQUEUE_PI always expect absolute time in CLOCK_REALTIME independent of the CLOCK_REALTIME flag. The flag was explicitely added to allow FUTEX_WAIT to hand in absolute time. > > diff --git a/kernel/futex.c b/kernel/futex.c > > index 33664f7..4bee915 100644 > > --- a/kernel/futex.c > > +++ b/kernel/futex.c > > @@ -3230,7 +3230,7 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val, > > return -EINVAL; > > > > t = timespec_to_ktime(ts); > > - if (cmd == FUTEX_WAIT) > > + if (cmd == FUTEX_WAIT && !(op & FUTEX_CLOCK_REALTIME)) > > t = ktime_add_safe(ktime_get(), t); > > tp = &t; > > } So this patch is correct and if the man page is unclear about it then we need to fix that. Thanks, tglx