From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752121AbcFWTxy (ORCPT ); Thu, 23 Jun 2016 15:53:54 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:32796 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751390AbcFWTxx (ORCPT ); Thu, 23 Jun 2016 15:53:53 -0400 Date: Thu, 23 Jun 2016 12:53:46 -0700 From: Darren Hart To: "Michael Kerrisk (man-pages)" Cc: Thomas Gleixner , Matthieu CASTET , linux-kernel@vger.kernel.org, Darren Hart , Peter Zijlstra , Davidlohr Bueso , Eric Dumazet Subject: Re: futex: Allow FUTEX_CLOCK_REALTIME with FUTEX_WAIT op Message-ID: <20160623195346.GC106079@f23x64.localdomain> References: <20160620162652.2c4619c3@perruche.parrot.biz> <20160623044828.GA3379@f23x64.localdomain> <520d203a-83ad-0277-79d5-2209d6853628@gmail.com> <20160623161643.GA106079@f23x64.localdomain> <8b6775b1-efee-beec-0ba8-136183742ca3@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8b6775b1-efee-beec-0ba8-136183742ca3@gmail.com> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 23, 2016 at 08:35:15PM +0200, Michael Kerrisk (man-pages) wrote: > Hi Darren, > > On 06/23/2016 06:16 PM, Darren Hart wrote: > > On Thu, Jun 23, 2016 at 03:40:36PM +0200, Thomas Gleixner wrote: > > > On Thu, 23 Jun 2016, Michael Kerrisk (man-pages) wrote: > > > > On 06/23/2016 09:18 AM, Thomas Gleixner wrote: > > > > Once upon a time, you told me the following: > > > > > > > > On 15 May 2014 at 16:14, Thomas Gleixner wrote: > > > > > On Thu, 15 May 2014, Michael Kerrisk (man-pages) wrote: > > > > > > And that universe would love to have your documentation of > > > > > > FUTEX_WAKE_BITSET and FUTEX_WAIT_BITSET ;-), > > > > > > > > > > I give you almost the full treatment, but I leave REQUEUE_PI to Darren > > > > > and FUTEX_WAKE_OP to Jakub. :) > > > > > [...] > > > > > FUTEX_CLOCK_REALTIME > > > > > > > > > > This option bit can be ored on the futex ops FUTEX_WAIT_BITSET > > > > > and FUTEX_WAIT_REQUEUE_PI > > > > > > > > > > If set the kernel treats the user space supplied timeout as > > > > > absolute time based on CLOCK_REALTIME. > > > > > > > > > > If not set the kernel treats the user space supplied timeout > > > > > as relative time. > > > > Unfortunately, I should have checked the code more carefully... > > > > > > Me too :) > > > > Seems to be going around... > > > > > > > > > Looking more carefully at the code, I see understand the situation > > > > is the following: > > > > > > > > FUTEX_LOCK_PI > > > > Always uses CLOCK_REALTIME > > > > 'timeout' is absolute > > > > > > Yes. > > > > > > > FUTEX_WAIT_REQUEUE_PI > > > > Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is > > > > determined by presence or absence of > > > > FUTEX_CLOCK_REALTIME flag > > > > 'timeout' is absolute > > > > > > Yes > > > > > > > FUTEX_WAIT_BITSET > > > > Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is > > > > determined by presence or absence of > > > > FUTEX_CLOCK_REALTIME flag > > > > 'timeout' is absolute > > > > > > Yes > > > > > > > FUTEX_WAIT > > > > Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is > > > > determined by presence or absence of > > > > FUTEX_CLOCK_REALTIME flag > > > > 'timeout' is relative > > > > > > Yes. > > > > > > > I've amended the man page to describe those details. > > > > OK, that confirms my question, timeout interpretation as relative or absolute is > > based on the op code, not the CLOCK flag. > > > > > > > > > > > The flag was explicitely added to allow FUTEX_WAIT to hand in absolute time. > > > > > > > > When you say that the "flag was added", which flag do you mean? Or, did you > > > > mean: "applying Matthieu's patch will allow FUTEX_WAIT to hand in absolute > > > > time". > > > > > > I didn't express myself clearly. When Darren added the support for > > > CLOCK_REALTIME to FUTEX_WAIT I think he wanted to add absolute timeout > > > support. Anything else does not make sense. > > > > I sent that patch because reading the new man page it struck me as strange that > > FUTEX_WAIT was restricted to CLOCK_MONOTONIC and the other op codes were not, > > especially since FUTEX_WAIT is a just FUTEX_WAIT_BITSET with the mask set to > > ALL. > > > > I didn't realize the impact to relative/absolute interpretation of the timeout > > value at the time. > > > > I think it was a mistake to introduce a change that made FUTEX_WAIT interpret > > the timeout differently based on the CLOCK flag, > > I'm missing something. Where does it do that? As far as I can tell FUTEX_WAIT > always interprets the clock as relative, regardless of presence/absence of > FUTEX_CLOCK_REALTIME? Am I missing something? No you're not. The code as it stands today is always relative, but it gets the base time from the wrong clock source in the case of FUTEX_CLOCK_REALTIME. I was stating that I think it would be a mistake to add absolute timeout to FUTEX_WAIT based on the FUTEX_CLOCK_REALTIME flag, which is how Thomas describes above his interpretation of my earlier change. -- Darren Hart Intel Open Source Technology Center