Re: Linux 2.4.10-pre11 -- __builtin_expect

[Andi Kleen]

"H. Peter Anvin" <[e-mail]> writes:

> Followup to:  <[e-mail]>
> By author:    Alan Cox <[e-mail]>
> In newsgroup: linux.dev.kernel
> > 
> > You need gcc 2.96 or higher to build the pre11 tree. I doubt that was
> > intentional. Basically rip out all use of __builtin_expect
> > 
> 
> Perhaps we should have a header which does #define __builtin_expect(X)
> if your gcc version is 2.91-95?

__builtin_expect() is ugly to use directly. In x86-64 I'm using slightly
higher level primitives (likely() and unlikely()), which make the intend
more clear.

I sent Linus a patch to move them into generic code some time ago, but it 
was ignored. Here is it again for 2.4.10pre11 with the generic code changed 
to use them. Please consider applying.

More comments:

For kernel debugging I can only recommend to compile with 
-fno-reorder-blocks. Gcc output gone through the basic block reordering pass
is *really* unreadable.

A lot of the unlikely()s seem to be in front of BUGs. If BUG was moved out 
of line and marked __attribute__((noreturn)) this could be avoided, because
the reordering pass would already do the right thing. I didn't do that 
change yet.

-Andi

--- mm/slab.c-LIKELY	Tue Sep 18 03:37:30 2001
+++ mm/slab.c	Tue Sep 18 11:26:43 2001
@@ -1230,7 +1230,7 @@
 	objp = slabp->s_mem + slabp->free*cachep->objsize;
 	slabp->free=slab_bufctl(slabp)[slabp->free];
 
-	if (__builtin_expect(slabp->free == BUFCTL_END, 0)) {
+	if (unlikely(slabp->free == BUFCTL_END)) {
 		list_del(&slabp->list);
 		list_add(&slabp->list, &cachep->slabs_full);
 	}
@@ -1264,11 +1264,11 @@
 								\
 	slabs_partial = &(cachep)->slabs_partial;		\
 	entry = slabs_partial->next;				\
-	if (__builtin_expect(entry == slabs_partial, 0)) {	\
+	if (unlikely(entry == slabs_partial)) {	\
 		struct list_head * slabs_free;			\
 		slabs_free = &(cachep)->slabs_free;		\
 		entry = slabs_free->next;			\
-		if (__builtin_expect(entry == slabs_free, 0))	\
+		if (unlikely(entry == slabs_free))	\
 			goto alloc_new_slab;			\
 		list_del(entry);				\
 		list_add(entry, slabs_partial);			\
@@ -1291,11 +1291,11 @@
 		/* Get slab alloc is to come from. */
 		slabs_partial = &(cachep)->slabs_partial;
 		entry = slabs_partial->next;
-		if (__builtin_expect(entry == slabs_partial, 0)) {
+		if (unlikely(entry == slabs_partial)) {
 			struct list_head * slabs_free;
 			slabs_free = &(cachep)->slabs_free;
 			entry = slabs_free->next;
-			if (__builtin_expect(entry == slabs_free, 0))
+			if (unlikely(entry == slabs_free))
 				break;
 			list_del(entry);
 			list_add(entry, slabs_partial);
@@ -1436,11 +1436,11 @@
 	/* fixup slab chains */
 	{
 		int inuse = slabp->inuse;
-		if (__builtin_expect(!--slabp->inuse, 0)) {
+		if (unlikely(!--slabp->inuse)) {
 			/* Was partial or full, now empty. */
 			list_del(&slabp->list);
 			list_add(&slabp->list, &cachep->slabs_free);
-		} else if (__builtin_expect(inuse == cachep->num, 0)) {
+		} else if (unlikely(inuse == cachep->num)) {
 			/* Was full. */
 			list_del(&slabp->list);
 			list_add(&slabp->list, &cachep->slabs_partial);
--- mm/vmscan.c-LIKELY	Tue Sep 18 03:37:30 2001
+++ mm/vmscan.c	Tue Sep 18 11:22:09 2001
@@ -335,7 +335,7 @@
 	while (__max_scan && (entry = lru->prev) != lru) {
 		struct page * page;
 
-		if (__builtin_expect(current->need_resched, 0)) {
+		if (unlikely(current->need_resched)) {
 			spin_unlock(&pagemap_lru_lock);
 			schedule();
 			spin_lock(&pagemap_lru_lock);
@@ -344,7 +344,7 @@
 
 		page = list_entry(entry, struct page, lru);
 
-		if (__builtin_expect(!PageInactive(page) && !PageActive(page), 0))
+		if (unlikely(!PageInactive(page) && !PageActive(page)))
 			BUG();
 
 		if (PageTestandClearReferenced(page)) {
@@ -363,7 +363,7 @@
 		list_del(entry);
 		list_add_tail(entry, &inactive_local_lru);
 
-		if (__builtin_expect(!memclass(page->zone, classzone), 0))
+		if (unlikely(!memclass(page->zone, classzone)))
 			continue;
 
 		__max_scan--;
@@ -380,7 +380,7 @@
 		 * The page is locked. IO in progress?
 		 * Move it to the back of the list.
 		 */
-		if (__builtin_expect(TryLockPage(page), 0))
+		if (unlikely(TryLockPage(page)))
 			continue;
 
 		if (PageDirty(page) && is_page_cache_freeable(page)) {
@@ -456,10 +456,10 @@
 			}
 		}
 
-		if (__builtin_expect(!page->mapping, 0))
+		if (unlikely(!page->mapping))
 			BUG();
 
-		if (__builtin_expect(!spin_trylock(&pagecache_lock), 0)) {
+		if (unlikely(!spin_trylock(&pagecache_lock))) {
 			/* we hold the page lock so the page cannot go away from under us */
 			spin_unlock(&pagemap_lru_lock);
 
@@ -479,7 +479,7 @@
 		}
 
 		/* point of no return */
-		if (__builtin_expect(!PageSwapCache(page), 1))
+		if (likely(!PageSwapCache(page)))
 			__remove_inode_page(page);
 		else
 			__delete_from_swap_cache(page);
--- mm/page_alloc.c-LIKELY	Tue Sep 18 03:37:30 2001
+++ mm/page_alloc.c	Tue Sep 18 11:26:44 2001
@@ -372,7 +372,7 @@
 		return page;
 
 	zone = zonelist->zones;
-	if (__builtin_expect(freed, 1)) {
+	if (likely(freed)) {
 		for (;;) {
 			zone_t *z = *(zone++);
 			if (!z)
--- include/linux/kernel.h-LIKELY	Tue Sep 18 11:12:20 2001
+++ include/linux/kernel.h	Tue Sep 18 11:30:58 2001
@@ -171,4 +171,14 @@
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
+
+/* This loses on a few early 2.96 snapshots, but hopefully nobody uses them anymore. */ 
+#if __GNUC__ > 2 || (__GNUC__ == 2 && _GNUC_MINOR__ == 96)
+#define likely(x)  __builtin_expect((x), !0) 
+#define unlikely(x)  __builtin_expect((x), 0) 
+#else
+#define likely(x) (x)
+#define unlikely(x) (x)
+#endif
+
 #endif