Messages in this thread Patch in this message | | | Subject | [PATCH -rt] get rid of unnecessary (un)likely's | From | Steven Rostedt <> | Date | Thu, 23 Mar 2006 15:27:48 -0500 |
| |
Thomas/Ingo
Looking at the code, I notice that there are a few likely and unlikely flags that really don't belong. Really there is two places, but for the rt_mutex and rt_sems, they are the same.
We have in the blocking of the lock:
if (unlikely(!waiter.task)) task_blocks_on_rt_mutex(lock, &waiter, 0 __IP__);
Where, the first time around the for loop it is true, and the next time around it is most likely false. So a fifty/fifty compare really shouldn't use a likely or unlikely.
The next place is in the slow unlocks. We have:
if (likely(!rt_mutex_has_waiters(lock))) { lock->owner = NULL; spin_unlock_irqrestore(&lock->wait_lock, flags); return; }
Now this would be true if cmpxchg wasn't available. But, if it is, then this would actually be an unlikely case. We would fail the fast unlock when we have waiters. The supplied patch just removes this altogether, but maybe it would be a good idea to have a little macro:
if (cmpxchg_rt_waiters(!rt_mutex_has_waiters(lock))) {
with:
#if defined(__HAVE_ARCH_CMPXCHG) ... # define cmpxchg_rt_waiters(x) unlikely(x) #else ... # define cmpxchg_rt_waiters(x) likely(x) #endif
Just a thought.
-- Steve
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Index: linux-2.6.16-rt6/kernel/rtmutex.c =================================================================== --- linux-2.6.16-rt6.orig/kernel/rtmutex.c 2006-03-23 15:10:57.000000000 -0500 +++ linux-2.6.16-rt6/kernel/rtmutex.c 2006-03-23 15:11:10.000000000 -0500 @@ -670,7 +670,7 @@ rt_lock_slowlock(struct rt_mutex *lock _ * when we have been woken up by the previous owner * but the lock got stolen by an higher prio task. */ - if (unlikely(!waiter.task)) + if (!waiter.task) task_blocks_on_rt_mutex(lock, &waiter, 0 __IP__); /* @@ -730,7 +730,7 @@ rt_lock_slowunlock(struct rt_mutex *lock rt_mutex_deadlock_account_unlock(current); - if (likely(!rt_mutex_has_waiters(lock))) { + if (!rt_mutex_has_waiters(lock)) { lock->owner = NULL; spin_unlock_irqrestore(&lock->wait_lock, flags); return; @@ -858,7 +858,7 @@ rt_mutex_slowlock(struct rt_mutex *lock, * when we have been woken up by the previous owner * but the lock got stolen by an higher prio task. */ - if (unlikely(!waiter.task)) { + if (!waiter.task) { ret = task_blocks_on_rt_mutex(lock, &waiter, deadlock_detect __IP__); if (ret == -EDEADLK) @@ -961,7 +961,7 @@ rt_mutex_slowunlock(struct rt_mutex *loc rt_mutex_deadlock_account_unlock(current); - if (likely(!rt_mutex_has_waiters(lock))) { + if (!rt_mutex_has_waiters(lock)) { lock->owner = NULL; spin_unlock_irqrestore(&lock->wait_lock, flags); return;
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |