[PATCH -rt] get rid of unnecessary (un)likely's

[Steven Rostedt]

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@xxxxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/