Skip to contentSkip to conversation

futex requeueing feature, futex-requeue-2.5.69-D3

PATCH Martin Wirth 3 participants 3 messages May 22, 2003

lore
Share & tools
lore
#0 lore raw
Quoted 3 lines · Ingo Molnar

all that is needed now is some actual review of the new APIs from the
conceptual angle (i've done that and i think they are okay, but more >eyes see more), so that we make sure these are good and we wont need to discard any aspect of them anytime soon.

What about adding an u32 flags field to each one of the new futex sys_calls. This gives you more freedom for future extensions without changing the API again. Possible uses may be:

- Specify the futexes to be mm-local: By using the pair mm* and vaddr as key it is possible to have process local futexes living on the same hash with the following advantages:
1. no page_table lock contention (I implemented mm-local futexes for my application after I noticed long latencies on SMP where a high prio tasks spun in futex_wake while another task was doing mmap/munmap on a second processor).
2. no vcache pollution (I guess 99% of all futexes will not be in shared memory)
3. Slightly faster, since no page pinning is needed

- Specify queueing or unqueueing in priority order

Martin

P.S.

By the way, your latest futex patch still contains the bogus if (!list_empty(&q.list)) conditional, that's always true since you hold the locks at this point and no one can have removed us from the list:

Review

Quoted 7 lines
	add_wait_queue(&q.waiters, &wait);
	set_current_state(TASK_INTERRUPTIBLE);
(quoted diff) +2 -1
-	if (!list_empty(&q.list))
+	if (!list_empty(&q.list)) {
+		unlock_futex_mm();
		time = schedule_timeout(time);
(quoted diff) +1 -0
+	}

Of course the test would be (and was) pretty necessary if you drop the locks before the get_user(...) call. And I must admit that I still can't see why you need to hold the locks across get_user. Even if it's save to do so at least every automatic code checker will bark at this point.

#1 lore raw
Quoted 10 lines · Martin Wirth

- Specify the futexes to be mm-local: By using the pair mm* and vaddr as key it is possible to have process local futexes living on the same hash with the following advantages:
1. no page_table lock contention (I implemented mm-local futexes for my application after I noticed long latencies on SMP where a high prio tasks spun in futex_wake while another task was doing mmap/munmap on a second processor).
2. no vcache pollution (I guess 99% of all futexes will not be in shared memory)
3. Slightly faster, since no page pinning is needed

there's a patch in the works to change futex hashing to not require page pinning, without making futexes process-local. This should also fix the FUTEX_FD problems.

basically the idea is that struct page remains to be a good hash whenever the page is present, and the swapout pte value is a good hash key when the page is swapped out. So we basically can make futexes swappable.

this gets rid of pinned pages and pagetable locking, without the need for API-level changes.

Quoted 3 lines

By the way, your latest futex patch still contains the bogus if (!list_empty(&q.list)) conditional, that's always true since you hold the locks at this point and no one can have removed us from the list:

ok, i'll fix this! I knew i missed something ...

Ingo

#2 lore raw
Quoted 8 lines · Ingo Molnar

there's a patch in the works to change futex hashing to not require page pinning, without making futexes process-local. This should also fix the FUTEX_FD problems.
basically the idea is that struct page remains to be a good hash whenever the page is present, and the swapout pte value is a good hash key when the page is swapped out. So we basically can make futexes swappable. this gets rid of pinned pages and pagetable locking, without the need for API-level changes.

Someone's already doing it? I'll back off if so.

-- wli