Re: [GIT PULL] futex fixlet

[Linus Torvalds]

On Fri, Dec 30, 2011 at 9:07 AM, Peter Zijlstra <a.p.zijlstra@xxxxxxxxx> wrote:
>
> So I _think_ you're completely right and we can simply kill the whole
> thing, but I've been trying very hard to forget everything kernel
> related for a week, and I really shouldn't kick-start my brain until
> somewhere next week.

Ok.

Ingo, I'd suggest you put a patch like that into -next, and mark it
for stable after it has gotten lots more testing.

Something like the appended *totally* untested code. It compiles, but
maybe there really is some "!page->mapping" case that could possibly
matter.

I can't see it, though - but we should definitely get some testing for
this before I'd put it in a release. Since we do that
"get_user_pages_fast()" and ask for a writable page, the result should
be as stable as it will ever get. There is still the race with
hugepage splitting, but at least that one has a comment.

                       Linus
 kernel/futex.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index ea87f4d2f455..8d2ee2f66418 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -310,21 +310,25 @@ again:
 	if (page != page_head) {
 		get_page(page_head);
 		put_page(page);
+		/* Avoid "unused label" for non-THP config */
+		if (0)
+			goto again;
 	}
 #endif
 
 	lock_page(page_head);
+
+	/*
+	 * If the page doesn't have a mapping associated with it,
+	 * this is either an anonymous page that was accessed RO
+	 * (so no COW etc) and we would just return EFAULT anyway,
+	 * or some special case page (ZERO_PAGE, gate area, special
+	 * mapping..).
+	 */
 	if (!page_head->mapping) {
 		unlock_page(page_head);
 		put_page(page_head);
-		/*
-		* ZERO_PAGE pages don't have a mapping. Avoid a busy loop
-		* trying to find one. RW mapping would have COW'd (and thus
-		* have a mapping) so this page is RO and won't ever change.
-		*/
-		if ((page_head == ZERO_PAGE(address)))
-			return -EFAULT;
-		goto again;
+		return -EFAULT;
 	}
 
 	/*