RE: [PATCH 2.5.69.bk9] Fix reference counts for pages (take 1) [was RE: condvar wakeups]

["Perez-Gonzalez, Inaky" <inaky perez-gonzalez intel com>]

> From: Saurabh Desai [mailto:sdesai austin ibm com]
> 
> "Perez-Gonzalez, Inaky" wrote:
> > @@ -242,11 +248,13 @@
> >                 if (this->page == page1 && this->offset == offset1) {
> >                         list_del_init(i);
> >                         __detach_vcache(&this->vcache);
> > +                       unpin_page(this->page);
> 
> Inaky, In the futex_requeue(), the unpin_page(this->page) should go in
> the "else" part. Because for the "if" part, where it wakes up the
> waiter, this->page==page1 and at the end it unpins page1 anyway. 
> So, it will unpin twice the same page. This unpin is needed for 
> the requeue part before it pins page2.

That's right - got it mixed up from rtfutexes ... grin - take 2
inlined.


diff -u linux/kernel/futex.c:1.1.1.1.2.4
linux/kernel/futex.c:1.1.1.1.2.2.2.4
--- linux/kernel/futex.c:1.1.1.1.2.4	Wed May 14 16:30:07 2003
+++ linux/kernel/futex.c	Thu May 15 12:15:07 2003
@@ -98,6 +98,13 @@
  *
  * Must be called with (and returns with) all futex-MM locks held.
  */
+static inline
+struct page *__pin_page_atomic (struct page *page)
+{
+	if (!PageReserved(page))
+		get_page(page);
+	return page;
+}
 static struct page *__pin_page(unsigned long addr)
 {
 	struct mm_struct *mm = current->mm;
@@ -108,11 +115,8 @@
 	 * Do a quick atomic lookup first - this is the fastpath.
 	 */
 	page = follow_page(mm, addr, 0);
-	if (likely(page != NULL)) {	
-		if (!PageReserved(page))
-			get_page(page);
-		return page;
-	}
+	if (likely(page != NULL))
+		__pin_page_atomic(page);
 
 	/*
 	 * No luck - need to fault in the page:
@@ -202,7 +206,9 @@
 	spin_lock(&futex_lock);
 
 	if (!list_empty(&q->list)) {
+		unpin_page(q->page);
 		q->page = new_page;
+		__pin_page_atomic(new_page);
 		list_del(&q->list);
 		list_add_tail(&q->list, head);
 	}
@@ -247,6 +253,8 @@
 				if (this->filp)
 					send_sigio(&this->filp->f_owner,
this->fd, POLL_IN);
 			} else {
+				unpin_page(this->page);
+				__pin_page_atomic (page2);
 				list_add_tail(i, head2);
 				__attach_vcache(&this->vcache, uaddr2,
current->mm, futex_vcache_callback);
 				this->offset = offset2;
@@ -362,7 +370,7 @@
 	/* Were we woken up anyway? */
 	if (!unqueue_me(&q))
 		ret = 0;
-	unpin_page(page);
+	unpin_page(q.page);
 
 	return ret;
 }

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)