futex priority based wakeup
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/4 futex priority based wakeup
Hi,
Today, all threads waiting for a given futex are woken in FIFO order (first waiter woken first) instead of priority order.
This patch makes use of plist (pirotity ordered lists) instead of simple list in futex_hash_bucket.
---
futex.c | 66 +++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 29 deletions(-)
---
---
kernel/futex.c +37 -29
@@ -106,12 +106,12 @@ struct futex_pi_state {* we can wake only the relevant ones (hashed queues may be shared). * * A futex_q has a woken state, just like tasks have TASK_RUNNING. -* It is considered woken when list_empty(&q->list) || q->lock_ptr == 0. +* It is considered woken when plist_node_empty(&q->list) || q->lock_ptr == 0. * The order of wakup is always to make the first condition true, then * wake up q->waiters, then make the second condition true. */ struct futex_q { -struct list_head list; +struct plist_node list;wait_queue_head_t waiters;/* Which hash list lock to use: */
@@ -133,8 +133,8 @@ struct futex_q {* Split the global futex_lock into every hash list lock. */ struct futex_hash_bucket { -spinlock_t lock; -struct list_head chain; +spinlock_t lock; +struct plist_head chain; };static struct futex_hash_bucket futex_queues[1 <<FUTEX_HASHBITS ];
@@ -465,13 +465,13 @@ lookup_pi_state(u32 uval, struct futex_h {struct futex_pi_state *pi_state =NULL ;struct futex_q *this, *next; -struct list_head *head; +struct plist_head *head;struct task_struct *p;pid_t pid; head = &hb->chain; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (match_futex (&this->key, &me->key)) {/* * Another waiter already exists - bump up
@@ -535,12 +535,12 @@ lookup_pi_state(u32 uval, struct futex_h*/ static void wake_futex (struct futex_q *q) { -list_del_init (&q->list); +plist_del (&q->list, &q->list.plist);if (q->filp)send_sigio (&q->filp->f_owner, q->fd,POLL_IN );/* * The lock in wake_up_all() is a crucial memory barrier after the -* list_del_init() and also before assigning to q->lock_ptr. +* plist_del() and also before assigning to q->lock_ptr. */ wake_up_all (&q->waiters);/*
@@ -653,7 +653,7 @@ static int futex_wake(u32 __user *uaddr, {struct futex_hash_bucket *hb;struct futex_q *this, *next; -struct list_head *head; +struct plist_head *head;union futex_key key;int ret;
@@ -667,7 +667,7 @@ static int futex_wake(u32 __user *uaddr,spin_lock (&hb->lock); head = &hb->chain; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (match_futex (&this->key, &key)) {if (this->pi_state) { ret = -EINVAL ;
@@ -695,7 +695,7 @@ futex_wake_op(u32 __user *uaddr1, u32 __ {union futex_key key1, key2;struct futex_hash_bucket *hb1, *hb2; -struct list_head *head; +struct plist_head *head;struct futex_q *this, *next;int ret, op_ret, attempt =0 ;
@@ -768,7 +768,7 @@ retry: head = &hb1->chain; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (match_futex (&this->key, &key1)) {wake_futex (this);if (++ret >= nr_wake)
@@ -780,7 +780,7 @@ retry: head = &hb2->chain; op_ret =0 ; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (match_futex (&this->key, &key2)) {wake_futex (this);if (++op_ret >= nr_wake2)
@@ -807,7 +807,7 @@ static int futex_requeue(u32 __user *uad {union futex_key key1, key2;struct futex_hash_bucket *hb1, *hb2; -struct list_head *head1; +struct plist_head *head1;struct futex_q *this, *next;int ret, drop_count =0 ;
@@ -856,7 +856,7 @@ static int futex_requeue(u32 __user *uad } head1 = &hb1->chain; -list_for_each_entry_safe (this, next, head1, list) { +plist_for_each_entry_safe (this, next, head1, list) {if (!match_futex (&this->key, &key1))continue ;if (++ret <= nr_wake) {
@@ -867,9 +867,13 @@ static int futex_requeue(u32 __user *uad* requeue. */ if (likely (head1 != &hb2->chain)) { -list_move_tail (&this->list, &hb2->chain); +plist_del (&this->list, &hb1->chain); +plist_add (&this->list, &hb2->chain); this->lock_ptr = &hb2->lock; - } +#ifdef CONFIG_DEBUG_PI_LIST + this->list.plist.lock = &hb2->lock; +#endif + } this->key = key2;get_key_refs (&key2); drop_count++;
@@ -914,7 +918,11 @@ queue_lock(struct futex_q *q, int fd, ststatic inline void __queue_me (struct futex_q *q,struct futex_hash_bucket *hb) { -list_add_tail (&q->list, &hb->chain); +plist_node_init (&q->list, current->normal_prio); +#ifdef CONFIG_DEBUG_PI_LIST + q->list.plist.lock = &hb->lock; +#endif +plist_add (&q->list, &hb->chain); q->task = current;spin_unlock (&hb->lock); }
@@ -969,8 +977,8 @@ static int unqueue_me(struct futex_q *q)spin_unlock (lock_ptr); goto retry; } -WARN_ON (list_empty (&q->list)); -list_del (&q->list); +WARN_ON (plist_node_empty (&q->list)); +plist_del (&q->list, &q->list.plist);BUG_ON (q->pi_state);
@@ -988,8 +996,8 @@ static int unqueue_me(struct futex_q *q)*/ static void unqueue_me_pi (struct futex_q *q,struct futex_hash_bucket *hb) { -WARN_ON (list_empty (&q->list)); -list_del (&q->list); +WARN_ON (plist_node_empty (&q->list)); +plist_del (&q->list, &q->list.plist);BUG_ON (!q->pi_state);free_pi_state (q->pi_state);
@@ -1082,10 +1090,10 @@ static int futex_wait(u32 __user *uaddr,__set_current_state (TASK_INTERRUPTIBLE );add_wait_queue (&q.waiters, &wait);/* -* !list_empty() is safe here without any lock. +* !plist_node_empty() is safe here without any lock. * q.lock_ptr != 0 is not safe, because of ordering against wakeup. */ -if (likely (!list_empty (&q.list))) +if (likely (!plist_node_empty (&q.list))) time =schedule_timeout (time);__set_current_state (TASK_RUNNING );
@@ -1358,7 +1366,7 @@ static int futex_unlock_pi(u32 __user *ustruct futex_hash_bucket *hb;struct futex_q *this, *next;u32 uval; -struct list_head *head; +struct plist_head *head;union futex_key key;int ret, attempt =0 ;
@@ -1409,7 +1417,7 @@ retry_locked:*/ head = &hb->chain; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (!match_futex (&this->key, &key))continue ; ret =wake_futex_pi (uaddr, uval, this);
@@ -1483,10 +1491,10 @@ static unsigned int futex_poll(struct fipoll_wait (filp, &q->waiters, wait);/* -* list_empty() is safe here without any lock. +* plist_node_empty() is safe here without any lock. * q->lock_ptr != 0 is not safe, because of ordering against wakeup. */ -if (list_empty (&q->list)) +if (plist_node_empty (&q->list)) ret =POLLIN |POLLRDNORM ;return ret;
@@ -1869,7 +1877,7 @@ static int __init init(void) }for (i =0 ; i <ARRAY_SIZE (futex_queues); i++) { -INIT_LIST_HEAD (&futex_queues[i].chain); +plist_head_init (&futex_queues[i].chain, &futex_queues[i].lock);spin_lock_init (&futex_queues[i].lock); }return 0 ;
Signature · 1 line
Pierre Peiffer
Quoted 3 lines · Pierre Peiffer
This patch makes use of plist (pirotity ordered lists) instead of simple list in
futex_hash_bucket.
I have never seen performance numbers for this. If it is punishing existing code in a measurable way I think it's not anacceptable default behavior.
Signature · 1 line
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
Quoted 5 lines · Ulrich Drepper
I have never seen performance numbers for this. If it is punishing existing code in a measurable way I think it's not anacceptable default behavior.
Here are some numbers. My test program measures the latency of pthread_broadcast with 1000 pthreads (all threads are blocked on pthread_cond_wait, the time is measured between the broadcast call and the last woken pthread).
Here are the average latencies after 5000 measures.
[only this patch is used, not the following.
The system is a dual Xeon 2.80GHz with HT enable]
First case: all threads are SCHED_OTHER
* with simple list:
Iterations=5000
Latency (us) min max avg stddev
3869 7400 6656.73 539.35
* with plist:
Iterations=5000
Latency (us) min max avg stddev
3684 7629 6787.97 479.41
Second case: all threads are SCHED_FIFO with priority equally distributed from priomin to priomax
* with simple list:
Iterations=5000
Latency (us) min max avg stddev
4548 7197 6656.85 463.30
* with plist:
Iterations=5000
Latency (us) min max avg stddev
8289 11752 9720.12 426.45
So, yes it (logically) has a cost, depending of the number of different priorities used, so it's specially measurable with real-time threads. With SCHED_OTHER, I suppose that the priorities are not be very distributed.
May be, supposing it makes sense to respect the priority order only for real-time pthreads, I can register all SCHED_OTHER threads to the same MAX_RT_PRIO priotity ? Or do you think this must be set behind a CONFIG* option ?
(Or finally not interesting enough for mainline ?)
Signature · 1 line
Pierre
Quoted 7 lines · Pierre Peiffer
Quoted 4 lines · Ulrich Drepper
I have never seen performance numbers for this. If it is punishing existing code in a measurable way I think it's not anacceptable default behavior.
May be, supposing it makes sense to respect the priority order only for real-time pthreads, I can register all
SCHED_OTHERthreads to the sameMAX_RT_PRIOpriotity ?
Moreover, the performance must be considered, sure, but after all, "man pthread_cond_broadcast" says:
<<
If more than one thread is blocked on a condition variable, the scheduling policy shall determine the order in which threads are unblocked.
Quoted 1 line
Quoted 1 line
... this is not true today ...
(of course, "shall" does not mean "mandatory", I know ;-) )
Signature · 1 line
Pierre
Quoted 9 lines · Pierre Peiffer
So, yes it (logically) has a cost, depending of the number of different priorities used, so it's specially measurable with real-time threads. With
SCHED_OTHER, I suppose that the priorities are not be very distributed.May be, supposing it makes sense to respect the priority order only for real-time pthreads, I can register all
SCHED_OTHERthreads to the sameMAX_RT_PRIOpriotity ?
Or do you think this must be set behind a CONFIG* option ? (Or finally not interesting enough for mainline ?)
As soon as there is at least one non-SCHED_OTHER thread among the waiters, there is no question about whether plist should be used or not, that's a correctness issue and if we want to conform to POSIX, we have to use that.
I guess Ulrich's question was mainly about performance differences with/without plist wakeup when all threads are SCHED_OTHER. I'd say for that a pure pthread_mutex_{lock,unlock} benchmark or even just a program which uses futex FUTEX_WAIT/FUTEX_WAKE in a bunch of threads would be better.
In the past we talked with Ingo about the possibilities here, one is use plist always and prove that it doesn't add measurable overhead over current FIFO (when only SCHED_OTHER is involved), the other possibility would be to start using FIFOs as before, but when the first non-SCHED_OTHER thread decides to wait on the futex, switch it to plist wakeup mode (convert the FIFO into a plist) and from that point on just use plist wakeups on it.
Jakub
Yes, I agree with all, that was what I have in mind too.
And by registering all SHED_OTHER threads with the same priority MAX_RT_PRIO, we have exactly this behavior, I think :
* the plist, when used with only one priority, behaves exactly as a simple list
(plist is a double simple list: first list contains all nodes sorted by priority, second list contains the first element of each "priority-based" sub-list of the first one):
Thus, when only one priority is used, there is nothing to sort at each add operation. (first list contains all elements by FIFO order, second list contains one element). So the overhead in this case is minimal and quasi-null.
* Now, if a SCHED_FIFO thread comes to the plist, its priority will be lower than MAX_RT_PRIO and it will be "naturally" sorted by prio order, and thus, it will be woken before the SCHED_OTHER threads because of its higher priority (i.e. lower prio-value in the kernel).
But there can be a performance impact when several processes use different futexes which have the same hash key.
In fact, the plist contains all waiters _of_all_futexes_ having the same hash key, not only the waiters of a given futex. This can be more a problem, if one process uses SCHED_FIFO threads, and the other SCHED_OTHER: the first will penalize the second... but even in this case, as the second has a lower priority, this can be acceptable, I think ?
Quoted 27 lines · Jakub Jelinek
Quoted 6 lines · Pierre Peiffer
So, yes it (logically) has a cost, depending of the number of different priorities used, so it's specially measurable with real-time threads. With
SCHED_OTHER, I suppose that the priorities are not be very distributed.May be, supposing it makes sense to respect the priority order only for real-time pthreads, I can register all
SCHED_OTHERthreads to the sameMAX_RT_PRIOpriotity ? Or do you think this must be set behind a CONFIG* option ?
(Or finally not interesting enough for mainline ?)As soon as there is at least one non-SCHED_OTHER thread among the waiters, there is no question about whether plist should be used or not, that's a correctness issue and if we want to conform to POSIX, we have to use that.
I guess Ulrich's question was mainly about performance differences with/without plist wakeup when all threads are
SCHED_OTHER. I'd say for that a pure pthread_mutex_{lock,unlock} benchmark or even just a program which uses futex FUTEX_WAIT/FUTEX_WAKE in a bunch of threads would be better.In the past we talked with Ingo about the possibilities here, one is use plist always and prove that it doesn't add measurable overhead over current FIFO (when only
SCHED_OTHERis involved), the other possibility would be to start using FIFOs as before, but when the first non-SCHED_OTHER thread decides to wait on the futex, switch it to plist wakeup mode (convert the FIFO into a plist) and from that point on just use plist wakeups on it.Jakub
Signature · 1 line
Pierre Peiffer
Quoted 5 lines · Pierre Peiffer
But there can be a performance impact when several processes use different futexes which have the same hash key.
In fact, the plist contains all waiters _of_all_futexes_ having the same hash key, not only the waiters of a given futex. This can be more a problem,
s/can be/is/
There are systems with thousands of active futexes, maybe tens of thousands. Not only is hash collision likely, it's also a matter of using and administering the plist. We have to make futexes less connected, not more. Now I definitely want to see real world tests first.
Signature · 1 line
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
Quoted 2 lines · Pierre Peiffer
Here are the average latencies after 5000 measures. [...]
Use something more realistic. I suggest using the Volano benchmark under your favorite JVM. I found it to be quite representative and you get a nice number you can show.
Signature · 1 line
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
kernel/futex.c struct futex_q { · line 137
Quoted 8 lines · Pierre Peiffer
kernel/futex.c +2
-2@@ -133,8 +133,8 @@ struct futex_q {* Split the global futex_lock into every hash list lock. */ struct futex_hash_bucket { -spinlock_t lock; -struct list_head chain; +spinlock_t lock; +struct plist_head chain;
Should have tabs between spinlock_t and lock , and plist_head and chain.. It looks like the original didn't, but as long as your cleaning up may as well get add them.
Daniel
Quoted 10 lines · Pierre Peiffer
kernel/futex.c +1
-1@@ -1358,7 +1366,7 @@ static int futex_unlock_pi(u32 __user *ustruct futex_hash_bucket *hb;struct futex_q *this, *next;u32 uval; -struct list_head *head; +struct plist_head *head;union futex_key key;int ret, attempt =0 ;
kernel/futex.c retry_locked: · line 1423
Quoted 8 lines
kernel/futex.c +1
-1@@ -1409,7 +1417,7 @@ retry_locked:*/ head = &hb->chain; -list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (!match_futex (&this->key, &key))continue ; ret =wake_futex_pi (uaddr, uval, this);
Is this really necessary? The rtmutex will priority sort the waiters when you enable priority inheritance. Inside the wake_futex_pi() it actually just pulls the new owner off another plist inside the the rtmutex structure.
Daniel
Quoted 14 lines · Daniel Walker
Quoted 13 lines · Pierre Peiffer
kernel/futex.c +1
-1@@ -1358,7 +1366,7 @@ static int futex_unlock_pi(u32 __user *ustruct futex_hash_bucket *hb;struct futex_q *this, *next;u32 uval; -struct list_head *head; +struct plist_head *head;union futex_key key;int ret, attempt =0 ;@@ -1409,7 +1417,7 @@ retry_locked:*/ head = &hb->chain;
Quoted 11 lines
kernel/futex.c retry_locked: · line 1423
Quoted 5 lines
kernel/futex.c +1
-1-list_for_each_entry_safe (this, next, head, list) { +plist_for_each_entry_safe (this, next, head, list) {if (!match_futex (&this->key, &key))continue ; ret =wake_futex_pi (uaddr, uval, this);Is this really necessary? The rtmutex will priority sort the waiters when you enable priority inheritance. Inside the
wake_futex_pi()it actually just pulls the new owner off another plist inside the the rtmutex structure.
Yes. ... necessary for non-PI-futex (ie "normal" futex)...
As the hash_bucket_list is used and common for both futex and PI-futex, yes, in case of PI_futex, the task is queued two times in two plist.
Signature · 1 line
Pierre
Quoted 10 lines · Pierre Peiffer
Quoted 5 lines
Is this really necessary? The rtmutex will priority sort the waiters when you enable priority inheritance. Inside the
wake_futex_pi()it actually just pulls the new owner off another plist inside the the rtmutex structure.Yes. ... necessary for non-PI-futex (ie "normal" futex)...
As the
hash_bucket_listis used and common for both futex and PI-futex, yes, in case ofPI_futex, the task is queued two times in two plist.
You could make them distinct .. Also Did you consider merging the PI path and the non-PI code path? then you could modify the rtmutex to toggle PI depending .
Daniel
PATCH 2/4 Make futex_wait() use an hrtimer for timeout
In reply to a message not in the archiveHi,
This patch modifies futex_wait() to use an hrtimer + schedule() in place ofschedule_timeout() in an RT kernel.
More details in the patch header.
--------------------------------------------------------------------------------
This patch modifies futex_wait() to use an hrtimer + schedule() in place ofschedule_timeout().
schedule_timeout() is tick based, therefore the timeout granularity is
the tick (1 ms, 4 ms or 10 ms depending on HZ). By using a high resolution timer for timeout wakeup, we can attain a much finer timeout granularity (in the microsecond range). This parallels what is already done for futex_lock_pi().
The timeout passed to the syscall is no longer converted to jiffies
and is therefore passed to do_futex() and futex_wait() as a timespec therefore keeping nanosecond resolution.
Also this removes the need to pass the nanoseconds timeout part tofutex_lock_pi() in val2.
In futex_wait(), if the timeout is zero then a regular schedule() is
performed. Otherwise, an hrtimer is fired before schedule() is called.
--- include/linux/futex.h | 2 - kernel/futex.c | 58 ++++++++++++++++++++++++++++++++------------------ kernel/futex_compat.c | 11 +-------- 3 files changed, 41 insertions(+), 30 deletions(-)
---
---
kernel/futex.c +38 -20
@@ -1008,7 +1008,7 @@ static void unqueue_me_pi(struct futex_qdrop_key_refs (&q->key); } -static int futex_wait (u32 __user *uaddr,u32 val,unsigned long time) +static int futex_wait (u32 __user *uaddr,u32 val,struct timespec *time) {struct task_struct *curr = current;DECLARE_WAITQUEUE (wait, curr);
@@ -1016,6 +1016,8 @@ static int futex_wait(u32 __user *uaddr,struct futex_q q;u32 uval;int ret; +struct hrtimer_sleeper t; +int rem =0 ; q.pi_state =NULL ; retry:
@@ -1093,8 +1095,31 @@ static int futex_wait(u32 __user *uaddr,* !plist_node_empty() is safe here without any lock. * q.lock_ptr != 0 is not safe, because of ordering against wakeup. */ -if (likely (!plist_node_empty (&q.list))) - time =schedule_timeout (time); +if (likely (!plist_node_empty (&q.list))) { +if (time->tv_sec ==0 && time->tv_nsec ==0 ) +schedule (); +else { +hrtimer_init (&t.timer,CLOCK_MONOTONIC ,HRTIMER_REL ); +hrtimer_init_sleeper (&t, current); + t.timer.expires =timespec_to_ktime (*time); + +hrtimer_start (&t.timer, t.timer.expires,HRTIMER_REL ); + +/* +* the timer could have already expired, in which +* case current would be flagged for rescheduling. +* Don't bother calling schedule. +*/ +if (likely (t.task)) +schedule (); + +hrtimer_cancel (&t.timer); + +/* Flag if a timeout occured */ + rem = (t.task ==NULL ); + } + } +__set_current_state (TASK_RUNNING );/*
@@ -1105,7 +1130,7 @@ static int futex_wait(u32 __user *uaddr,/* If we were woken (and unqueued), we succeeded, whatever. */ if (!unqueue_me (&q))return 0 ; -if (time ==0 ) +if (rem)return -ETIMEDOUT ;/* * We expect signal_pending(current), but another thread may
@@ -1127,8 +1152,8 @@ static int futex_wait(u32 __user *uaddr,* if there are waiters then it will block, it does PI, etc. (Due to * races the kernel might see a 0 value of the futex too.) */ -static int futex_lock_pi (u32 __user *uaddr,int detect,unsigned long sec, -long nsec,int trylock) +static int futex_lock_pi (u32 __user *uaddr,int detect,struct timespec *time, +int trylock) {struct hrtimer_sleeper timeout, *to =NULL ;struct task_struct *curr = current;
@@ -1140,11 +1165,11 @@ static int futex_lock_pi(u32 __user *uadif (refill_pi_state_cache ())return -ENOMEM ; -if (sec !=MAX_SCHEDULE_TIMEOUT ) { +if (time->tv_sec || time->tv_nsec) { to = &timeout;hrtimer_init (&to->timer,CLOCK_REALTIME ,HRTIMER_ABS );hrtimer_init_sleeper (to, current); - to->timer.expires =ktime_set (sec, nsec); + to->timer.expires =timespec_to_ktime (*time); } q.pi_state =NULL ;
@@ -1780,7 +1805,7 @@ void exit_robust_list(struct task_struct } } -long do_futex (u32 __user *uaddr,int op,u32 val,unsigned long timeout, +long do_futex (u32 __user *uaddr,int op,u32 val,struct timespec *timeout,u32 __user *uaddr2,u32 val2,u32 val3) {int ret;
@@ -1806,13 +1831,13 @@ long do_futex(u32 __user *uaddr, int op, ret =futex_wake_op (uaddr, uaddr2, val, val2, val3);break ;case FUTEX_LOCK_PI : - ret =futex_lock_pi (uaddr, val, timeout, val2,0 ); + ret =futex_lock_pi (uaddr, val, timeout,0 );break ;case FUTEX_UNLOCK_PI : ret =futex_unlock_pi (uaddr);break ;case FUTEX_TRYLOCK_PI : - ret =futex_lock_pi (uaddr,0 , timeout, val2,1 ); + ret =futex_lock_pi (uaddr,0 , timeout,1 );break ;default : ret = -ENOSYS ;
@@ -1825,8 +1850,7 @@ asmlinkage long sys_futex(u32 __user *ua struct timespec __user *utime, u32 __user *uaddr2, u32 val3) { - struct timespec t; -unsigned long timeout =MAX_SCHEDULE_TIMEOUT ; + struct timespec t = {.tv_sec = 0, .tv_nsec = 0};u32 val2 =0 ;if (utime && (op ==FUTEX_WAIT || op ==FUTEX_LOCK_PI )) {
@@ -1834,12 +1858,6 @@ asmlinkage long sys_futex(u32 __user *uareturn -EFAULT ;if (!timespec_valid (&t))return -EINVAL ; -if (op ==FUTEX_WAIT ) - timeout =timespec_to_jiffies (&t) +1 ; -else { - timeout = t.tv_sec; - val2 = t.tv_nsec; - } }/* * requeue parameter in 'utime' if op == FUTEX_REQUEUE.
@@ -1847,7 +1865,7 @@ asmlinkage long sys_futex(u32 __user *uaif (op ==FUTEX_REQUEUE || op ==FUTEX_CMP_REQUEUE ) val2 = (u32 ) (unsigned long ) utime; -return do_futex (uaddr, op, val, timeout, uaddr2, val2, val3); +return do_futex (uaddr, op, val, &t, uaddr2, val2, val3); } static int futexfs_get_sb(struct file_system_type *fs_type,
kernel/futex_compat.c +2 -9
@@ -141,8 +141,7 @@ asmlinkage long compat_sys_futex(u32 __u struct compat_timespec __user *utime, u32 __user *uaddr2, u32 val3) { - struct timespec t; -unsigned long timeout =MAX_SCHEDULE_TIMEOUT ; + struct timespec t = {.tv_sec = 0, .tv_nsec = 0};int val2 =0 ;if (utime && (op ==FUTEX_WAIT || op ==FUTEX_LOCK_PI )) {
@@ -150,15 +149,9 @@ asmlinkage long compat_sys_futex(u32 __ureturn -EFAULT ;if (!timespec_valid (&t))return -EINVAL ; -if (op ==FUTEX_WAIT ) - timeout =timespec_to_jiffies (&t) +1 ; -else { - timeout = t.tv_sec; - val2 = t.tv_nsec; - } }if (op ==FUTEX_REQUEUE || op ==FUTEX_CMP_REQUEUE ) val2 = (int ) (unsigned long ) utime; -return do_futex (uaddr, op, val, timeout, uaddr2, val2, val3); +return do_futex (uaddr, op, val, &t, uaddr2, val2, val3); }
include/linux/futex.h +1 -1
@@ -94,7 +94,7 @@ struct robust_list_head {#define ROBUST_LIST_LIMIT 2048#ifdef __KERNEL__ -long do_futex (u32 __user *uaddr,int op,u32 val,unsigned long timeout, +long do_futex (u32 __user *uaddr,int op,u32 val,struct timespec *timeout,u32 __user *uaddr2,u32 val2,u32 val3); extern int
Signature · 1 line
Pierre Peiffer
PATCH 3/4 futex_requeue_pi optimization
In reply to a message not in the archiveHi,
This patch provides the futex_requeue_pi functionality.
This provides an optimization, already used for (normal) futexes, to be used for PI-futexes.
This optimization is currently used by the glibc in pthread_broadcast, when using "normal" mutexes. With futex_requeue_pi, it can be used with PRIO_INHERIT mutexes too.
---
include/linux/futex.h | 8 kernel/futex.c | 559 +++++++++++++++++++++++++++++++++++++++++++----- kernel/futex_compat.c | 3 kernel/rtmutex.c | 41 --- kernel/rtmutex_common.h | 34 ++ 5 files changed, 557 insertions(+), 88 deletions(-)
---
---
include/linux/futex.h +7 -1
@@ -15,6 +15,7 @@#define FUTEX_LOCK_PI 6#define FUTEX_UNLOCK_PI 7#define FUTEX_TRYLOCK_PI 8 +#define FUTEX_CMP_REQUEUE_PI 9/* * Support for robust futexes: the kernel cleans up held futexes at
@@ -83,9 +84,14 @@ struct robust_list_head {#define FUTEX_OWNER_DIED 0x40000000/* +* Some processes have been requeued on this PI-futex +*/ +#define FUTEX_WAITER_REQUEUED 0x20000000 + +/* * The rest of the robust-futex field is for the TID: */ -#define FUTEX_TID_MASK 0x3fffffff +#define FUTEX_TID_MASK 0x0fffffff/* * This limit protects against a deliberately circular list.
kernel/futex.c +505 -54
730 lines not on this page · raw message
kernel/rtmutex.c +9 -32
71 lines not on this page · raw message
kernel/rtmutex_common.h +34 -0
@@ -113,6 +113,29 @@ static inline unsigned long rt_mutex_own }/* +* We can speed up the acquire/release, if the architecture +* supports cmpxchg and if there's no debugging state to be set up +*/ +#if defined(__HAVE_ARCH_CMPXCHG) && !defined(CONFIG_DEBUG_RT_MUTEXES ) +# define rt_mutex_cmpxchg (l,c,n) (cmpxchg(&l->owner, c, n) == c) +static inline void mark_rt_mutex_waiters (struct rt_mutex *lock) +{ +unsigned long owner, *p = (unsigned long *) &lock->owner; + +do { + owner = *p; + }while (cmpxchg (p, owner, owner |RT_MUTEX_HAS_WAITERS ) != owner); +} +#else +# define rt_mutex_cmpxchg (l,c,n) (0) +static inline void mark_rt_mutex_waiters (struct rt_mutex *lock) +{ + lock->owner = (struct task_struct *) + ((unsigned long )lock->owner |RT_MUTEX_HAS_WAITERS ); +} +#endif + +/* * PI-futex support (proxy locking functions, etc.): */ extern struct task_struct *rt_mutex_next_owner (struct rt_mutex *lock);
@@ -120,4 +143,15 @@ extern void rt_mutex_init_proxy_locked(s struct task_struct *proxy_owner);extern void rt_mutex_proxy_unlock (struct rt_mutex *lock,struct task_struct *proxy_owner); + +extern void rt_mutex_set_owner (struct rt_mutex *lock,struct task_struct *owner, +unsigned long mask); +extern void __rt_mutex_adjust_prio (struct task_struct *task); +extern int rt_mutex_adjust_prio_chain (struct task_struct *task, +int deadlock_detect, +struct rt_mutex *orig_lock, +struct rt_mutex_waiter *orig_waiter, +struct task_struct *top_task); +extern void remove_waiter (struct rt_mutex *lock, +struct rt_mutex_waiter *waiter);#endif
kernel/futex_compat.c +2 -1
@@ -150,7 +150,8 @@ asmlinkage long compat_sys_futex(u32 __uif (!timespec_valid (&t))return -EINVAL ; } -if (op ==FUTEX_REQUEUE || op ==FUTEX_CMP_REQUEUE ) +if (op ==FUTEX_REQUEUE || op ==FUTEX_CMP_REQUEUE + || op ==FUTEX_CMP_REQUEUE_PI ) val2 = (int ) (unsigned long ) utime;return do_futex (uaddr, op, val, &t, uaddr2, val2, val3);
Signature · 1 line
Pierre Peiffer
Re: [PATCH 2.6.20-rc4 3/4] futex_requeue_pi optimization
Quoted 3 lines · Pierre Peiffer
This provides an optimization, already used for (normal) futexes, to be used for
PI-futexes.
So, this patch implements requeuing from a non-PI futex to a PI futex? That's the bare minimum needed. What about PI to PI?
Signature · 1 line
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
Re: [PATCH 2.6.20-rc4 3/4] futex_requeue_pi optimization
Quoted 2 lines · Ulrich Drepper
So, this patch implements requeuing from a non-PI futex to a PI futex?
Yes.
Quoted 1 line
That's the bare minimum needed. What about PI to PI?
I may miss something, but I don't think there is a need for that.
Currently, futex_requeue is (only) used in pthread_cond_broadcast to requeue some threads from an internal futex (futex1) to another futex (futex2, which is the futex behind the cond_mutex)
futex1 does not need to be a PI-futex, I think.
Signature · 1 line
Pierre
Re: [PATCH 2.6.20-rc4 3/4] futex_requeue_pi optimization
Quoted 1 line · Pierre Peiffer
I may miss something, but I don't think there is a need for that.
Yes, I know, I was asking about it only for completeness. Maybe there will be a reason to have it some day.
Signature · 1 line
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
RFC PATCH 4/4 sys_futex64 : allows 64bit futexes
In reply to a message not in the archiveHi,
This latest patch is an adaptation of the sys_futex64 syscall provided in -rt patch (originally written by Ingo). It allows the use of 64bit futex.
I have re-worked most of the code to avoid the duplication of the code.
It does not provide the functionality for all architectures, and thus, it can not be applied "as is".
But, again, feedbacks and comments are welcome.
--- include/asm-x86_64/futex.h | 113 ++++++++++++++++++++ include/asm-x86_64/unistd.h | 4 include/linux/futex.h | 5 include/linux/syscalls.h | 3 kernel/futex.c | 247 ++++++++++++++++++++++++++++++-------------- kernel/futex_compat.c | 3 kernel/sys_ni.c | 1 7 files changed, 299 insertions(+), 77 deletions(-)
---
---
include/asm-x86_64/futex.h +113 -0
@@ -41,6 +41,39 @@ "=&r" (tem) \ : "r" (oparg), "i" (-EFAULT), "m" (*uaddr), "1" (0)) +#define __futex_atomic_op1_64 (insn, ret, oldval, uaddr, oparg) \ + __asm__ __volatile ( \ +"1: " insn "\n" \ +"2: .section .fixup,\"ax\"\n\ +3: movq %3, %1\n\ + jmp 2b\n\ + .previous\n\ + .section __ex_table,\"a\"\n\ + .align 8\n\ + .quad 1b,3b\n\ + .previous" \ + : "=r" (oldval), "=r" (ret), "=m" (*uaddr) \ + : "i" (-EFAULT), "m" (*uaddr), "0" (oparg), "1" (0)) + +#define __futex_atomic_op2_64 (insn, ret, oldval, uaddr, oparg) \ + __asm__ __volatile ( \ +"1: movq %2, %0\n\ + movq %0, %3\n" \ + insn "\n" \ +"2: " LOCK_PREFIX "cmpxchgq %3, %2\n\ + jnz 1b\n\ +3: .section .fixup,\"ax\"\n\ +4: movq %5, %1\n\ + jmp 3b\n\ + .previous\n\ + .section __ex_table,\"a\"\n\ + .align 8\n\ + .quad 1b,4b,2b,4b\n\ + .previous" \ + : "=&a" (oldval), "=&r" (ret), "=m" (*uaddr), \ + "=&r" (tem) \ + : "r" (oparg), "i" (-EFAULT), "m" (*uaddr), "1" (0)) +static inline int futex_atomic_op_inuser (int encoded_op,int __user *uaddr) {
@@ -95,6 +128,60 @@ futex_atomic_op_inuser (int encoded_op, }static inline int +futex_atomic_op_inuser64 (int encoded_op,u64 __user *uaddr) +{ +int op = (encoded_op >>28 ) &7 ; +int cmp = (encoded_op >>24 ) &15 ; +u64 oparg = (encoded_op <<8 ) >>20 ; +u64 cmparg = (encoded_op <<20 ) >>20 ; +u64 oldval =0 , ret, tem; + +if (encoded_op & (FUTEX_OP_OPARG_SHIFT <<28 )) + oparg =1 << oparg; + +if (!access_ok (VERIFY_WRITE , uaddr,sizeof (u64))) +return -EFAULT ; + +inc_preempt_count (); + +switch (op) { +case FUTEX_OP_SET : +__futex_atomic_op1_64 ("xchgq %0, %2" , ret, oldval, uaddr, oparg); +break ; +case FUTEX_OP_ADD : +__futex_atomic_op1_64 (LOCK_PREFIX "xaddq %0, %2" , ret, oldval, + uaddr, oparg); +break ; +case FUTEX_OP_OR : +__futex_atomic_op2_64 ("orq %4, %3" , ret, oldval, uaddr, oparg); +break ; +case FUTEX_OP_ANDN : +__futex_atomic_op2_64 ("andq %4, %3" , ret, oldval, uaddr, ~oparg); +break ; +case FUTEX_OP_XOR : +__futex_atomic_op2_64 ("xorq %4, %3" , ret, oldval, uaddr, oparg); +break ; +default : + ret = -ENOSYS ; + } + +dec_preempt_count (); + +if (!ret) { +switch (cmp) { +case FUTEX_OP_CMP_EQ : ret = (oldval == cmparg);break ; +case FUTEX_OP_CMP_NE : ret = (oldval != cmparg);break ; +case FUTEX_OP_CMP_LT : ret = (oldval < cmparg);break ; +case FUTEX_OP_CMP_GE : ret = (oldval >= cmparg);break ; +case FUTEX_OP_CMP_LE : ret = (oldval <= cmparg);break ; +case FUTEX_OP_CMP_GT : ret = (oldval > cmparg);break ; +default : ret = -ENOSYS ; + } + } +return ret; +} + +static inline int futex_atomic_cmpxchg_inatomic (int __user *uaddr,int oldval,int newval) {if (!access_ok (VERIFY_WRITE , uaddr,sizeof (int )))
@@ -121,5 +208,31 @@ futex_atomic_cmpxchg_inatomic(int __userreturn oldval; } +static inline u64 +futex_atomic_cmpxchg_inatomic64 (u64 __user *uaddr,u64 oldval,u64 newval) +{ +if (!access_ok (VERIFY_WRITE , uaddr,sizeof (u64))) +return -EFAULT ; + + __asm__ __volatile__( +"1: " LOCK_PREFIX "cmpxchgq %3, %1 \n" + +"2: .section .fixup, \"ax\" \n" +"3: mov %2, %0 \n" +" jmp 2b \n" +" .previous \n" + +" .section __ex_table, \"a\" \n" +" .align 8 \n" +" .quad 1b,3b \n" +" .previous \n" + + :"=a" (oldval),"=m" (*uaddr) + :"i" (-EFAULT ),"r" (newval),"0" (oldval) + :"memory" + ); + +return oldval; +}#endif #endif
include/asm-x86_64/unistd.h +3 -1
@@ -619,8 +619,10 @@ __SYSCALL(__NR_sync_file_range, sys_sync __SYSCALL(__NR_vmsplice, sys_vmsplice)#define __NR_move_pages 279 __SYSCALL(__NR_move_pages, sys_move_pages) +#define __NR_futex64 280 +__SYSCALL(__NR_futex64, sys_futex64) -#define __NR_syscall_max __NR_move_pages +#define __NR_syscall_max __NR_futex64#ifndef __NO_STUBS#define __ARCH_WANT_OLD_READDIR
include/linux/syscalls.h +3 -0
@@ -178,6 +178,9 @@ asmlinkage long sys_set_tid_address(intasmlinkage long sys_futex (u32 __user *uaddr,int op,u32 val,struct timespec __user *utime,u32 __user *uaddr2,u32 val3); +asmlinkage long sys_futex64 (u64 __user *uaddr,int op,u64 val, +struct timespec __user *utime,u64 __user *uaddr2, +u64 val3);asmlinkage long sys_init_module (void __user *umod,unsigned long len,const char __user *uargs);
kernel/futex.c +174 -73
567 lines not on this page · raw message
kernel/sys_ni.c +1 -0
@@ -41,6 +41,7 @@ cond_syscall(sys_sendmsg);cond_syscall (sys_recvmsg);cond_syscall (sys_socketcall);cond_syscall (sys_futex); +cond_syscall (sys_futex64);cond_syscall (compat_sys_futex);cond_syscall (sys_set_robust_list);cond_syscall (compat_sys_set_robust_list);
include/linux/futex.h +3 -2
@@ -100,8 +100,9 @@ struct robust_list_head {#define ROBUST_LIST_LIMIT 2048#ifdef __KERNEL__ -long do_futex (u32 __user *uaddr,int op,u32 val,struct timespec *timeout, -u32 __user *uaddr2,u32 val2,u32 val3); +long do_futex (unsigned long __user *uaddr,int op,unsigned long val, +struct timespec *timeout,unsigned long __user *uaddr2, +unsigned long val2,unsigned long val3,int futex64);extern int handle_futex_death (u32 __user *uaddr,struct task_struct *curr,int pi);
kernel/futex_compat.c +2 -1
@@ -154,5 +154,6 @@ asmlinkage long compat_sys_futex(u32 __u || op == FUTEX_CMP_REQUEUE_PI) val2 = (int ) (unsigned long ) utime; -return do_futex (uaddr, op, val, &t, uaddr2, val2, val3); +return do_futex ((unsigned long __user *)uaddr, op, val, &t, + (unsigned long __user *)uaddr2, val2, val3,0 ); }
Signature · 1 line
Pierre Peiffer
Re: [PATCH 2.6.20-rc4 4/4][RFC] sys_futex64 : allows 64bit futexes
Quoted 2 lines · On Tue, 09 Jan 2007 17:25:26 +0100 Pierre Peiffer <[email protected]>…
static inline int
+futex_atomic_op_inuser64 (intencoded_op, u64__user*uaddr)
Your email client performs space-stuffing. Please see if you can turn that off. (It's fixable at my end with s/^ /^/g but it's a nuisance).
Quoted 4 lines
include/asm-x86_64/futex.h +4
-0+{ +int op = (encoded_op >>28 ) &7 ; +int cmp = (encoded_op >>24 ) &15 ; +u64 oparg = (encoded_op <<8 ) >>20 ;
include/asm-x86_64/futex.h futex_atomic_op_inuser (int encoded_op, · line 145
Quoted 10 lines
include/asm-x86_64/futex.h +10
-0+u64 cmparg = (encoded_op <<20 ) >>20 ; +u64 oldval =0 , ret, tem; + +if (encoded_op & (FUTEX_OP_OPARG_SHIFT <<28 )) + oparg =1 << oparg; + +if (!access_ok (VERIFY_WRITE , uaddr,sizeof (u64))) +return -EFAULT ; + +inc_preempt_count ();
What is that open-coded, uncommented inc_preempt_count() doing in there?
Re: [PATCH 2.6.20-rc4 4/4][RFC] sys_futex64 : allows 64bit futexes
In reply to Pierre PeifferQuoted 10 lines · Pierre Peiffer
This latest patch is an adaptation of the
sys_futex64syscall provided in -rt
patch (originally written by Ingo). It allows the use of 64bit futex.I have re-worked most of the code to avoid the duplication of the code.
It does not provide the functionality for all architectures, and thus, it can
not be applied "as is".
But, again, feedbacks and comments are welcome.
Why do you support all operations for 64-bit futexes?
IMHO PI futexes don't make sense for 64-bit futexes, PI futexes have hardcoded bit layout of the 32-bit word. Similarly, FUTEX_WAKE is not really necessary for 64-bit futexes, 32-bit futex's FUTEX_WAKE can wake it equally well (it never reads anything, all it cares is about the futex's address). Similarly, I don't see a need for FUTEX_WAKE_OP (and this could simplify the patch quite a lot, no need to change asm*/futex.h headers at all).
All that's needed is 64-bit FUTEX_WAIT and perhaps FUTEX_CMP_REQUEUE.
Jakub