public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhltc@us.ibm.com>
To: Michel Lespinasse <walken@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	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 21:40:55 -0800	[thread overview]
Message-ID: <4B0388E7.5080704@us.ibm.com> (raw)
In-Reply-To: <20091118042128.GC23808@google.com>

Michel Lespinasse wrote:

> One difficulty with adaptive spinning is that we want to avoid deadlocks.
> If two threads end up spinning in-kernel waiting for each other, we better
> have preemption enabled... or detect and deal with the situation somehow.

This is really only a problem for SCHED_FIFO tasks right? (SCHED_OTHER 
should get scheduled() out when CFS deems they've exhausted their fair 
share). Real-Time tasks typically should be using PI anyway as adaptive 
locking is non-deterministic and doesn't provide for PI. So I'm not sure 
how critical this problem is in practice.

> Also one aspect I dislike is that this would impose a given format on the
> futex for storing the TID.

We do have a precedent for this with robust as well as PI futexes.

  I would prefer if there were several bits available
> in the futex for userspace to do whatever they want. 8 bits would likely
> be enough, which leaves 24 for the TID - enough for us, but I have no idea
> if that's good enough for upstream inclusion. It that's not possible,
> one possible compromise could be:

And we already use two of those bits for OWNER_DIED and FUTEX_WAITERS. 
Perhaps you just have to choose between your own value scheme and 
adaptive spinning (sounds horribly limiting as I'm typing this...).

> 
> - userspace passes a TID (which it extracted from the futex value; but kernel
>   does not necessarily know how)
> - kernel spins until that TID goes to sleep, or the futex value is not equal
>   to val or setval anymore
> - if val != setval and the futex value is val, set it to setval
> - if the futex valus is setval, block, otherwise -EWOULDBLOCK.
> 
> If the lock got stolen from a different thread, userspace can decide to
> retry with or without adaptive spinning.

I'll think on this a bit more...

> 
> That would be the most generic interface I can think of, though it's
> starting to be a LOT of parameters - actually, too many to pass through
> the _syscall6 interface.
> 
> 
> I also like Darren's suggestion to do a FUTEX_SET_WAIT_REQUEUE_PI,
> but it's hitting the same 'too many parameters' limitation as well :/

We don't use val2 for FUTEX_WAIT_REQUEUE_PI, so we should be able to use 
that for setval.


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

  reply	other threads:[~2009-11-18  5:40 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-17  7:46 [PATCH] futex: add FUTEX_SET_WAIT operation 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
2009-11-18  4:21     ` Michel Lespinasse
2009-11-18  5:40       ` Darren Hart [this message]
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=4B0388E7.5080704@us.ibm.com \
    --to=dvhltc@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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