The NetBSD Project

CVS log for src/sys/kern/sys_futex.c

[BACK] Up to [cvs.NetBSD.org] / src / sys / kern

Request diff between arbitrary revisions


Default branch: MAIN


Revision 1.18 / (download) - annotate - [select for diffs], Thu Apr 21 12:05:13 2022 UTC (9 months, 1 week ago) by riastradh
Branch: MAIN
CVS Tags: netbsd-10-base, netbsd-10, bouyer-sunxi-drm-base, bouyer-sunxi-drm, HEAD
Changes since 1.17: +4 -4 lines
Diff to previous 1.17 (colored)

futex(9): Convert membar_enter/exit to membar_acquire/release.

No functional change -- this is just in an illustrative comment!

Revision 1.17 / (download) - annotate - [select for diffs], Sat Apr 9 23:38:33 2022 UTC (9 months, 2 weeks ago) by riastradh
Branch: MAIN
Changes since 1.16: +4 -4 lines
Diff to previous 1.16 (colored)

sys: Use membar_release/acquire around reference drop.

This just goes through my recent reference count membar audit and
changes membar_exit to membar_release and membar_enter to
membar_acquire -- this should make everything cheaper on most CPUs
without hurting correctness, because membar_acquire is generally
cheaper than membar_enter.

Revision 1.16 / (download) - annotate - [select for diffs], Sat Mar 12 15:32:32 2022 UTC (10 months, 2 weeks ago) by riastradh
Branch: MAIN
Changes since 1.15: +8 -2 lines
Diff to previous 1.15 (colored)

sys: Membar audit around reference count releases.

If two threads are using an object that is freed when the reference
count goes to zero, we need to ensure that all memory operations
related to the object happen before freeing the object.

Using an atomic_dec_uint_nv(&refcnt) == 0 ensures that only one
thread takes responsibility for freeing, but it's not enough to
ensure that the other thread's memory operations happen before the
freeing.

Consider:

	  Thread A			  Thread B
	obj->foo = 42;			obj->baz = 73;
	mumble(&obj->bar);		grumble(&obj->quux);
	/* membar_exit(); */		/* membar_exit(); */
	atomic_dec -- not last		atomic_dec -- last
					/* membar_enter(); */
					KASSERT(invariant(obj->foo,
					    obj->bar));
					free_stuff(obj);

The memory barriers ensure that

	obj->foo = 42;
	mumble(&obj->bar);

in thread A happens before

	KASSERT(invariant(obj->foo, obj->bar));
	free_stuff(obj);

in thread B.  Without them, this ordering is not guaranteed.

So in general it is necessary to do

	membar_exit();
	if (atomic_dec_uint_nv(&obj->refcnt) != 0)
		return;
	membar_enter();

to release a reference, for the `last one out hit the lights' style
of reference counting.  (This is in contrast to the style where one
thread blocks new references and then waits under a lock for existing
ones to drain with a condvar -- no membar needed thanks to mutex(9).)

I searched for atomic_dec to find all these.  Obviously we ought to
have a better abstraction for this because there's so much copypasta.
This is a stop-gap measure to fix actual bugs until we have that.  It
would be nice if an abstraction could gracefully handle the different
styles of reference counting in use -- some years ago I drafted an
API for this, but making it cover everything got a little out of hand
(particularly with struct vnode::v_usecount) and I ended up setting
it aside to work on psref/localcount instead for better scalability.

I got bored of adding #ifdef __HAVE_ATOMIC_AS_MEMBAR everywhere, so I
only put it on things that look performance-critical on 5sec review.
We should really adopt membar_enter_preatomic/membar_exit_postatomic
or something (except they are applicable only to atomic r/m/w, not to
atomic_load/store_*, making the naming annoying) and get rid of all
the ifdefs.

Revision 1.12.4.7 / (download) - annotate - [select for diffs], Wed Nov 3 14:49:21 2021 UTC (14 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12.4.6: +11 -9 lines
Diff to previous 1.12.4.6 (colored) to branchpoint 1.12 (colored) next main 1.13 (colored)

Cherry-pick this sys_futex.c revision and associated changes:

revision 1.13
date: 2021-09-28 08:05:42 -0700;  author: thorpej;  state: Exp;  lines: +11 -9;  commitid: FPndTp2ZDjYuyJaD;
futex_release_all_lwp(): No need to pass the "tid" argument separately; that
is a vestige of an older version of the code.  Also, move a KASSERT() that
both futex_release_all_lwp() call sites had inside of futex_release_all_lwp()
itself.

...so make this easier to test this sys_futex.c with trunk.

Revision 1.12.4.6 / (download) - annotate - [select for diffs], Mon Nov 1 08:40:16 2021 UTC (14 months, 4 weeks ago) by chs
Branch: thorpej-futex2
Changes since 1.12.4.5: +3 -3 lines
Diff to previous 1.12.4.5 (colored) to branchpoint 1.12 (colored)

merge rev. 1.15 from HEAD:
fix a typo in compare_futex_key().

Revision 1.15 / (download) - annotate - [select for diffs], Mon Nov 1 08:35:17 2021 UTC (14 months, 4 weeks ago) by chs
Branch: MAIN
Changes since 1.14: +3 -3 lines
Diff to previous 1.14 (colored)

fix a typo in compare_futex_key().

Revision 1.14 / (download) - annotate - [select for diffs], Thu Oct 21 13:21:54 2021 UTC (15 months, 1 week ago) by andvar
Branch: MAIN
Changes since 1.13: +3 -3 lines
Diff to previous 1.13 (colored)

fix various typos, mainly in comments, but also in man pages and log messages.

Revision 1.13 / (download) - annotate - [select for diffs], Tue Sep 28 15:05:42 2021 UTC (16 months ago) by thorpej
Branch: MAIN
Changes since 1.12: +11 -9 lines
Diff to previous 1.12 (colored)

futex_release_all_lwp(): No need to pass the "tid" argument separately; that
is a vestige of an older version of the code.  Also, move a KASSERT() that
both futex_release_all_lwp() call sites had inside of futex_release_all_lwp()
itself.

Revision 1.12.4.5 / (download) - annotate - [select for diffs], Sat Aug 7 01:22:33 2021 UTC (17 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12.4.4: +21 -11 lines
Diff to previous 1.12.4.4 (colored) to branchpoint 1.12 (colored)

The return values for FUTEX_REQUEUE and FUTEX_CMP_REQUEUE are different,
but we weren't doing to the right thing.

FUTEX_REQUEUE returns the number of waiters awakened.

FUTEX_CMP_REQUEUE returns the number of waiters awakenend plus the
number of waiters requeued (and it is an exercise for the caller to
calculate the number requeued, if it cares).

Revision 1.12.4.4 / (download) - annotate - [select for diffs], Fri Aug 6 23:53:53 2021 UTC (17 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12.4.3: +18 -15 lines
Diff to previous 1.12.4.3 (colored) to branchpoint 1.12 (colored)

Isolate knowledge of the union-ness of futex_key to where it's declared.

Revision 1.12.4.3 / (download) - annotate - [select for diffs], Fri Aug 6 18:23:57 2021 UTC (17 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12.4.2: +3 -3 lines
Diff to previous 1.12.4.2 (colored) to branchpoint 1.12 (colored)

Correct a comment.

Revision 1.12.4.2 / (download) - annotate - [select for diffs], Thu Aug 5 23:23:50 2021 UTC (17 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12.4.1: +19 -4 lines
Diff to previous 1.12.4.1 (colored) to branchpoint 1.12 (colored)

At the end of futex_wait(), when sleepq_block() returns 0, we would like
to assert that l->l_futex == NULL, because all of the code paths that awaken
a blocked thread in sys_futex.c itself clear l->l_futex.  Unfortunately,
there are certain received-a-signal situations (e.g. SIGKILL) where
sleepq_block() will not return an error after being awakened by the signal,
rendering this assertion too strong.

So, rather than going down the rabbit hole of reasoning out and altering
long-standing behavior of the signals code, just don't assert there and
treat a zero-return from sleepq_block() as an aborted futex wait if
l->l_futex != NULL.

(Thanks chs@ for helping chase this one down.)

Revision 1.12.4.1 / (download) - annotate - [select for diffs], Thu Aug 5 23:14:21 2021 UTC (17 months, 3 weeks ago) by thorpej
Branch: thorpej-futex2
Changes since 1.12: +649 -444 lines
Diff to previous 1.12 (colored)

Bring over just the futex sleepq infrastructure changes from thorpej-futex
to a new branch based on current HEAD.  This contains only the fixes for
the priority problems, and is intended to finish debugging those changes
(without the new extensions).

Revision 1.11.6.1 / (download) - annotate - [select for diffs], Sun Aug 1 22:42:38 2021 UTC (17 months, 4 weeks ago) by thorpej
Branch: thorpej-i2c-spi-conf
Changes since 1.11: +3 -2 lines
Diff to previous 1.11 (colored) next main 1.12 (colored)

Sync with HEAD.

Revision 1.12 / (download) - annotate - [select for diffs], Wed Jul 21 06:35:45 2021 UTC (18 months, 1 week ago) by skrll
Branch: MAIN
CVS Tags: thorpej-i2c-spi-conf2-base, thorpej-i2c-spi-conf2, thorpej-i2c-spi-conf-base, thorpej-futex2-base, thorpej-cfargs2-base, thorpej-cfargs2
Branch point for: thorpej-futex2
Changes since 1.11: +3 -2 lines
Diff to previous 1.11 (colored)

need <sys/param.h> for COHERENCY_UNIT

Minor KNF along the way.

Revision 1.11.2.2 / (download) - annotate - [select for diffs], Sat Apr 3 21:52:20 2021 UTC (21 months, 3 weeks ago) by thorpej
Branch: thorpej-futex
Changes since 1.11.2.1: +12 -4 lines
Diff to previous 1.11.2.1 (colored) to branchpoint 1.11 (colored) next main 1.12 (colored)

futex_func_wait(): If TIMER_ABSTIME, sanity check that the deadline
provided by the caller is not ridiculous.

Revision 1.11.2.1 / (download) - annotate - [select for diffs], Sun Nov 1 15:16:43 2020 UTC (2 years, 2 months ago) by thorpej
Branch: thorpej-futex
Changes since 1.11: +1155 -443 lines
Diff to previous 1.11 (colored)

Major overfaul of futex implemention:
- Use sleepqs directly, rather than using condition variables and
  separate wait queues / strutures.  By doing this, and using the
  standard mechanism for keeping sleepqs sorted by priority, we
  acn ensure that the highest priority waiters will be awakened,
  rather than naively awakening in FIFO order.
- As part of the data structure re-organization, struct lwp gains
  "l_futex" (the futex an LWP is blocked on) and "l_futex_wakesel"
  (the futex wake selector bitset) fields (and loses l___rsvd1).
  Plese note the special locking considerations for these fields
  documented in the comments.
- Add the notion of a "futex class".  This is prep work for eventually
  supporting the FUTEX_*_PI operations, as well as some future NetBSD
  extensions to the futex interface.
- Add a preliminary implementation of the first of those NetBSD extensions,
  FUTEX_NETBSD_RW_WAIT and FUTEX_NETBSD_RW_HANDOFF.  These are designed
  to implement reader/writer locks with direct-handoff to the correct
  priority thread(s) (real-time read-waiters need to have priority over
  non-real-time write-waiters).  NOTE: this is currently disabled due to
  a mysterious panic that haasn't yet been tracked down.
- Add some SDT probes to aid in debugging.

Revision 1.11 / (download) - annotate - [select for diffs], Tue May 5 15:25:18 2020 UTC (2 years, 8 months ago) by riastradh
Branch: MAIN
CVS Tags: thorpej-futex-base, thorpej-cfargs-base, thorpej-cfargs, cjep_sun2x-base1, cjep_sun2x-base, cjep_sun2x, cjep_staticlib_x-base1, cjep_staticlib_x-base, cjep_staticlib_x
Branch point for: thorpej-i2c-spi-conf, thorpej-futex
Changes since 1.10: +52 -18 lines
Diff to previous 1.10 (colored)

Revert "Use cv_timedwaitclock_sig in futex."

Turned out to break things; we'll do this another way.

Revision 1.10 / (download) - annotate - [select for diffs], Tue May 5 15:23:32 2020 UTC (2 years, 8 months ago) by riastradh
Branch: MAIN
Changes since 1.9: +1 -5 lines
Diff to previous 1.9 (colored)

Revert "Make sure futex waits never return ERESTART."

Part of redoing the timedwaitclock changes, which were buggy and
committed a little too fast.

Revision 1.9 / (download) - annotate - [select for diffs], Sun May 3 01:26:39 2020 UTC (2 years, 8 months ago) by riastradh
Branch: MAIN
Changes since 1.8: +7 -3 lines
Diff to previous 1.8 (colored)

Make sure futex waits never return ERESTART.

If the user had passed in a relative timeout, this would have the
effect of waiting for the full relative time repeatedly, without
regard for how much time had elapsed during the wait before a signal.

In principle this may not be necessary for absolute timeouts or
indefinite timeouts, but it's not clear there's an advantage; we do
the same for various other syscalls like nanosleep.

Perhaps in the future we can arrange to keep the state of how much
time had elapsed when we restart like Linux does, but that's a much
more ambitious change.

Revision 1.8 / (download) - annotate - [select for diffs], Sun May 3 01:25:48 2020 UTC (2 years, 8 months ago) by riastradh
Branch: MAIN
Changes since 1.7: +18 -52 lines
Diff to previous 1.7 (colored)

Use cv_timedwaitclock_sig in futex.

Possible fix for hangs observed with Java under Linux emulation.

Revision 1.7 / (download) - annotate - [select for diffs], Tue Apr 28 17:27:03 2020 UTC (2 years, 9 months ago) by riastradh
Branch: MAIN
Changes since 1.6: +3 -3 lines
Diff to previous 1.6 (colored)

Make FUTEX_WAIT_BITSET(bitset=0) fail with EINVAL to match Linux.

Revision 1.6 / (download) - annotate - [select for diffs], Tue Apr 28 16:22:25 2020 UTC (2 years, 9 months ago) by riastradh
Branch: MAIN
Changes since 1.5: +13 -2 lines
Diff to previous 1.5 (colored)

Fix waiting on a zero bitset.

The logic in futex_wait assumes there are two paths out:

1. Error (signal or timeout), in which case we take ourselves off the
   queue.

2. Wakeup, in which case the waker takes us off the queue.

But if the user does FUTEX_WAIT_BITSET(bitset=0), as in the
futex_wait_pointless_bitset test, then we will never even go to
sleep, so there will be nobody to wake us as in (2), but it's not an
error as in (1) either.  As a result, we're left on the queue.

Instead, don't bother with any of the wait machinery in that case.
This does not actually match Linux semantics -- Linux returns EINVAL
if bitset is zero.  But let's make sure this passes the releng test
rig as the tests are written now, and then fix both the logic and the
tests -- this is a candidate fix for:

lib/libc/sys/t_futex_ops (277/847): 20 test cases
    futex_basic_wait_wake_private: [6.645189s] Passed.
    futex_basic_wait_wake_shared: [6.572692s] Passed.
    futex_cmp_requeue: [4.624082s] Passed.
    futex_requeue: [4.427191s] Passed.
    futex_wait_pointless_bitset: [0.202865s] Passed.
    futex_wait_timeout_deadline: [ 9074.4164779] panic: TAILQ_INSERT_TAIL 0xffff000056a1ad48 /tmp/bracket/build/2020.04.28.03.00.23-evbarm-aarch64/src/sys/kern/sys_futex.c:826
[ 9074.4340691] cpu0: Begin traceback...
[ 9074.4340691] trace fp ffffc0004ceffb40
[ 9074.4340691] fp ffffc0004ceffb60 vpanic() at ffffc000004aac58 netbsd:vpanic+0x160
[ 9074.4441432] fp ffffc0004ceffbd0 panic() at ffffc000004aad4c netbsd:panic+0x44
[ 9074.4441432] fp ffffc0004ceffc60 futex_wait_enqueue() at ffffc000004b7710 netbsd:futex_wait_enqueue+0x138
[ 9074.4555795] fp ffffc0004ceffc80 futex_func_wait.part.5() at ffffc000004b82f4 netbsd:futex_func_wait.part.5+0x17c
[ 9074.4660518] fp ffffc0004ceffd50 do_futex() at ffffc000004b8cd8 netbsd:do_futex+0x1d0
[ 9074.4660518] fp ffffc0004ceffdf0 sys___futex() at ffffc000004b9078 netbsd:sys___futex+0x50

Revision 1.5 / (download) - annotate - [select for diffs], Tue Apr 28 00:54:24 2020 UTC (2 years, 9 months ago) by riastradh
Branch: MAIN
Changes since 1.4: +16 -28 lines
Diff to previous 1.4 (colored)

Rename futex_get -> futex_lookup_create.  Remove futex_put.

Just use futex_rele instead of futex_put.  There may once have been a
method to the madness this alias in an early draft but there is no
longer.

No functional change; all names are private to sys_futex.c.

Revision 1.4 / (download) - annotate - [select for diffs], Mon Apr 27 23:54:43 2020 UTC (2 years, 9 months ago) by riastradh
Branch: MAIN
Changes since 1.3: +70 -19 lines
Diff to previous 1.3 (colored)

Fix races in aborted futex waits.

- Re-check the wake condition in futex_wait in the event of error.
  => Otherwise, if futex_wait times out in cv_timedwait_sig but
     futex_wake wakes it while cv_timedwait_sig is still trying to
     reacquire fw_lock, the wake would be incorrectly accounted.

- Fold futex_wait_abort into futex_wait so it happens atomically.
  => Otherwise, if futex_wait times out and release fw_lock, then,
     before futex_wait_abort reacquires the lock and removes it from
     the queue, the waiter could be woken by futex_wake.  But once we
     enter futex_wait_abort, the decision to abort is final, so the
     wake would incorrectly accounted.

- In futex_wait_abort, mark each waiter aborting while we do the lock
  dance, and skip over aborting waiters in futex_wake and
  futex_requeue.
  => Otherwise, futex_wake might move it to a new futex while
     futex_wait_abort has released all the locks -- but
     futex_wait_abort still has the old futex, so TAILQ_REMOVE will
     cross the streams and bad things will happen.

- In futex_wait_abort, release the futex we moved the waiter off.
  => Otherwise, we would leak the futex reference acquired by
     futex_func_wait, in the event of aborting.  (For normal wakeups,
     futex_wake releases the reference on our behalf.)

- Consistently use futex_wait_dequeue rather than TAILQ_REMOVE so that
  all changes to fw_futex and the waiter queue are isolated to
  futex_wait_enqueue/dequeue and happen together.

Patch developed with and tested by thorpej@.

Revision 1.3 / (download) - annotate - [select for diffs], Mon Apr 27 05:28:17 2020 UTC (2 years, 9 months ago) by thorpej
Branch: MAIN
Changes since 1.2: +2 -6 lines
Diff to previous 1.2 (colored)

We would have bigger problems if PAGE_SIZE were < sizeof(int).  Remove
a CTASSERT() that can't be evaluated at compile-time on all platforms.

Revision 1.2 / (download) - annotate - [select for diffs], Sun Apr 26 21:04:46 2020 UTC (2 years, 9 months ago) by mlelstv
Branch: MAIN
Changes since 1.1: +3 -3 lines
Diff to previous 1.1 (colored)

fix DIAGNOSTIC build

Revision 1.1 / (download) - annotate - [select for diffs], Sun Apr 26 18:53:33 2020 UTC (2 years, 9 months ago) by thorpej
Branch: MAIN

Add a NetBSD native futex implementation, mostly written by riastradh@.
Map the COMPAT_LINUX futex calls to the native ones.

This form allows you to request diff's between any two revisions of a file. You may select a symbolic revision name using the selection box or you may type in a numeric name using the type-in text box.




CVSweb <webmaster@jp.NetBSD.org>