mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Michel Lespinasse <walken@google.com>,
	Darren Hart <dvhltc@us.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] futex: add FUTEX_SET_WAIT operation
Date: Tue, 17 Nov 2009 07:24:09 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.01.0911170708110.9384@localhost.localdomain> (raw)
In-Reply-To: <1258447807.7816.20.camel@laptop>



On Tue, 17 Nov 2009, Peter Zijlstra wrote:
>
> (long quote for Linus' benefit, added an old patch to the tail)

Both look conceptually sane to me.

The FUTEX_SET_WAIT concept seems well-defined, although it sounds more 
like a FUTEX_CMPXCHG_WAIT to me than a "SET" operation. I'm not entirely 
sure that we really want to do the CMPXCHG in the kernel rather than in 
user space, since lock stealing generally isn't a problem, but I don't 
think it's _wrong_ to add this concept.

In fact, CMPXCHG is generally seen to be the "fundamental" base for 
implementing locking, so in that sense it makes perfect sense to have it 
as a FUTEX model.

That said, I personally think the adaptive wait model is (a) more likely 
to fix many performance issues and (b) a bit more high-level concept, so I 
like Peter's patch too, but I don't see that the patches would really be 
mutually exclusive.

Of course, it's possible that Michel's performance problem is fixed by the 
adaptive approach too, in which case the FUTEX_SET_WAIT (or _CMPXCHG_WAIT) 
patch is just fundamentally less interesting. But some people do need 
fairness - even when it's bad for performance - so...

One thing that does strike me is that _if_ we want to do both interfaces, 
then I would assume that we quite likely also want to have an adaptive 
version of the FUTEX_SET|CMPXCHG_WAIT thing. Which perhaps implies that 
the "ADAPTIVE" part should be a bitflag in the command value?

		Linus

  reply	other threads:[~2009-11-17 15:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-17  7:46 Michel Lespinasse
2009-11-17  8:18 ` Ingo Molnar
2009-11-17  8:55   ` Peter Zijlstra
2009-11-17 16:16     ` Darren Hart
2009-11-18  3:37       ` Michel Lespinasse
2009-11-18  5:29         ` Darren Hart
2009-11-24 14:39         ` [PATCH 0/3] perf bench: Add new benchmark for futex subsystem Hitoshi Mitake
2009-11-24 14:39         ` [PATCH 1/3] perf bench: Add wrappers for atomic operation of GCC Hitoshi Mitake
2009-11-24 16:20           ` Darren Hart
2009-11-26  5:44             ` Hitoshi Mitake
2009-11-24 14:39         ` [PATCH 2/3] perf bench: Add new files for futex performance test Hitoshi Mitake
2009-11-24 16:33           ` Darren Hart
2009-11-26  5:53             ` Hitoshi Mitake
2009-11-26  5:56               ` [PATCH] futextest: Make locktest() in harness.h more general Hitoshi Mitake
2009-11-24 14:39         ` [PATCH 3/3] perf bench: Fix misc files to build files related to futex Hitoshi Mitake
2009-11-18 22:13       ` [PATCH] futex: add FUTEX_SET_WAIT operation Michel Lespinasse
2009-11-19  6:51         ` Darren Hart
2009-11-19 17:03         ` Darren Hart
     [not found]           ` <8d20b11a0911191325u49624854u6132594f13b0718c@mail.gmail.com>
2009-11-19 23:13             ` Darren Hart
2009-11-21  2:36               ` Michel Lespinasse
2009-11-23 17:21                 ` Darren Hart
2009-11-17 17:24     ` Ingo Molnar
2009-11-17 17:27       ` Darren Hart
2009-11-18  1:49       ` Hitoshi Mitake
2009-11-17  8:50 ` Peter Zijlstra
2009-11-17 15:24   ` Linus Torvalds [this message]
2009-11-18  4:21     ` Michel Lespinasse
2009-11-18  5:40       ` Darren Hart
2009-11-30 22:09   ` Darren Hart
2009-12-03  6:55   ` [PATCH] futex: add FUTEX_SET_WAIT operation (and ADAPTIVE) Darren Hart
2009-11-17 17:22 ` [PATCH] futex: add FUTEX_SET_WAIT operation Darren Hart
2009-11-18  3:29   ` Michel Lespinasse
2009-11-18  0:13 ` Darren Hart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.01.0911170708110.9384@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=dvhltc@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=walken@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®