Kyle McMartin cbdb312
From 799c10559d60f159ab2232203f222f18fa3c4a5f Mon Sep 17 00:00:00 2001
Kyle McMartin cbdb312
From: Linus Torvalds <torvalds@linux-foundation.org>
Kyle McMartin cbdb312
Date: Fri, 15 Oct 2010 11:09:28 -0700
Kyle McMartin cbdb312
Subject: [PATCH] De-pessimize rds_page_copy_user
Kyle McMartin cbdb312
Kyle McMartin cbdb312
Don't try to "optimize" rds_page_copy_user() by using kmap_atomic() and
Kyle McMartin cbdb312
the unsafe atomic user mode accessor functions.  It's actually slower
Kyle McMartin cbdb312
than the straightforward code on any reasonable modern CPU.
Kyle McMartin cbdb312
Kyle McMartin cbdb312
Back when the code was written (although probably not by the time it was
Kyle McMartin cbdb312
actually merged, though), 32-bit x86 may have been the dominant
Kyle McMartin cbdb312
architecture.  And there kmap_atomic() can be a lot faster than kmap()
Kyle McMartin cbdb312
(unless you have very good locality, in which case the virtual address
Kyle McMartin cbdb312
caching by kmap() can overcome all the downsides).
Kyle McMartin cbdb312
Kyle McMartin cbdb312
But these days, x86-64 may not be more populous, but it's getting there
Kyle McMartin cbdb312
(and if you care about performance, it's definitely already there -
Kyle McMartin cbdb312
you'd have upgraded your CPU's already in the last few years).  And on
Kyle McMartin cbdb312
x86-64, the non-kmap_atomic() version is faster, simply because the code
Kyle McMartin cbdb312
is simpler and doesn't have the "re-try page fault" case.
Kyle McMartin cbdb312
Kyle McMartin cbdb312
People with old hardware are not likely to care about RDS anyway, and
Kyle McMartin cbdb312
the optimization for the 32-bit case is simply buggy, since it doesn't
Kyle McMartin cbdb312
verify the user addresses properly.
Kyle McMartin cbdb312
Kyle McMartin cbdb312
Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
Kyle McMartin cbdb312
Acked-by: Andrew Morton <akpm@linux-foundation.org>
Kyle McMartin cbdb312
Cc: stable@kernel.org
Kyle McMartin cbdb312
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Kyle McMartin cbdb312
---
Kyle McMartin cbdb312
 net/rds/page.c |   27 +++++++--------------------
Kyle McMartin cbdb312
 1 files changed, 7 insertions(+), 20 deletions(-)
Kyle McMartin cbdb312
Kyle McMartin cbdb312
diff --git a/net/rds/page.c b/net/rds/page.c
Kyle McMartin cbdb312
index 595a952..1dfbfea 100644
Kyle McMartin cbdb312
--- a/net/rds/page.c
Kyle McMartin cbdb312
+++ b/net/rds/page.c
Kyle McMartin cbdb312
@@ -57,30 +57,17 @@ int rds_page_copy_user(struct page *page, unsigned long offset,
Kyle McMartin cbdb312
 	unsigned long ret;
Kyle McMartin cbdb312
 	void *addr;
Kyle McMartin cbdb312
 
Kyle McMartin cbdb312
-	if (to_user)
Kyle McMartin cbdb312
+	addr = kmap(page);
Kyle McMartin cbdb312
+	if (to_user) {
Kyle McMartin cbdb312
 		rds_stats_add(s_copy_to_user, bytes);
Kyle McMartin cbdb312
-	else
Kyle McMartin cbdb312
+		ret = copy_to_user(ptr, addr + offset, bytes);
Kyle McMartin cbdb312
+	} else {
Kyle McMartin cbdb312
 		rds_stats_add(s_copy_from_user, bytes);
Kyle McMartin cbdb312
-
Kyle McMartin cbdb312
-	addr = kmap_atomic(page, KM_USER0);
Kyle McMartin cbdb312
-	if (to_user)
Kyle McMartin cbdb312
-		ret = __copy_to_user_inatomic(ptr, addr + offset, bytes);
Kyle McMartin cbdb312
-	else
Kyle McMartin cbdb312
-		ret = __copy_from_user_inatomic(addr + offset, ptr, bytes);
Kyle McMartin cbdb312
-	kunmap_atomic(addr, KM_USER0);
Kyle McMartin cbdb312
-
Kyle McMartin cbdb312
-	if (ret) {
Kyle McMartin cbdb312
-		addr = kmap(page);
Kyle McMartin cbdb312
-		if (to_user)
Kyle McMartin cbdb312
-			ret = copy_to_user(ptr, addr + offset, bytes);
Kyle McMartin cbdb312
-		else
Kyle McMartin cbdb312
-			ret = copy_from_user(addr + offset, ptr, bytes);
Kyle McMartin cbdb312
-		kunmap(page);
Kyle McMartin cbdb312
-		if (ret)
Kyle McMartin cbdb312
-			return -EFAULT;
Kyle McMartin cbdb312
+		ret = copy_from_user(addr + offset, ptr, bytes);
Kyle McMartin cbdb312
 	}
Kyle McMartin cbdb312
+	kunmap(page);
Kyle McMartin cbdb312
 
Kyle McMartin cbdb312
-	return 0;
Kyle McMartin cbdb312
+	return ret ? -EFAULT : 0;
Kyle McMartin cbdb312
 }
Kyle McMartin cbdb312
 EXPORT_SYMBOL_GPL(rds_page_copy_user);
Kyle McMartin cbdb312
 
Kyle McMartin cbdb312
-- 
Kyle McMartin cbdb312
1.7.3.2
Kyle McMartin cbdb312