[PATCH] copy_strings speedup

This is the first of three patches which reduce the amount of
kmap/kunmap traffic on highmem machines.

The workload which was tested was RAM-only dbench.  This is dominated
by copy_*_user() costs.

The three patches speed up my 4xPIII by 3%

The three patches speed up a 16P NUMA-Q by 100 to 150%

The first two patches (copy_strings and pagecache reads) speed up an
8-way by 15%.  I expect that all three patches will speed up the 8-way
by 40%.

Some of the benefit is from reduced pressure on kmap_lock.  Most of it
is from reducing the number of global TLB invalidations.

This patch fixes up copy_strings().  copy_strings does a huge amount of
kmapping.  Martin Bligh has noted that across a kernel compile this
function is the second or third largest user of kmaps in the kernel.

The fix is pretty simple: just hang onto the previous kmap as we we go
around the loop.  It reduces the number of kmappings from copy_strings
by a factor of 30.

diff --git a/fs/exec.c b/fs/exec.c
index dca2263cf2b03b4432bc374817f99e5d97b8cb93..cc1e09c5aa74a29616cc67192fcfcb476917cb7f 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -185,25 +185,39 @@ static int count(char ** argv, int max)
  */
 int copy_strings(int argc,char ** argv, struct linux_binprm *bprm) 
 {
+       struct page *kmapped_page = NULL;
+       char *kaddr = NULL;
+       int ret;
+
        while (argc-- > 0) {
                char *str;
                int len;
                unsigned long pos;
 
-               if (get_user(str, argv+argc) || !(len = strnlen_user(str, bprm->p)))
-                       return -EFAULT;
-               if (bprm->p < len) 
-                       return -E2BIG; 
+               if (get_user(str, argv+argc) ||
+                               !(len = strnlen_user(str, bprm->p))) {
+                       ret = -EFAULT;
+                       goto out;
+               }
+
+               if (bprm->p < len)  {
+                       ret = -E2BIG;
+                       goto out;
+               }
 
                bprm->p -= len;
                /* XXX: add architecture specific overflow check here. */ 
-
                pos = bprm->p;
+
+               /*
+                * The only sleeping function which we are allowed to call in
+                * this loop is copy_from_user().  Otherwise, copy_user_state
+                * could get trashed.
+                */
                while (len > 0) {
-                       char *kaddr;
                        int i, new, err;
-                       struct page *page;
                        int offset, bytes_to_copy;
+                       struct page *page;
 
                        offset = pos % PAGE_SIZE;
                        i = pos/PAGE_SIZE;
@@ -212,32 +226,44 @@ int copy_strings(int argc,char ** argv, struct linux_binprm *bprm)
                        if (!page) {
                                page = alloc_page(GFP_HIGHUSER);
                                bprm->page[i] = page;
-                               if (!page)
-                                       return -ENOMEM;
+                               if (!page) {
+                                       ret = -ENOMEM;
+                                       goto out;
+                               }
                                new = 1;
                        }
-                       kaddr = kmap(page);
 
+                       if (page != kmapped_page) {
+                               if (kmapped_page)
+                                       kunmap(kmapped_page);
+                               kmapped_page = page;
+                               kaddr = kmap(kmapped_page);
+                       }
                        if (new && offset)
                                memset(kaddr, 0, offset);
                        bytes_to_copy = PAGE_SIZE - offset;
                        if (bytes_to_copy > len) {
                                bytes_to_copy = len;
                                if (new)
-                                       memset(kaddr+offset+len, 0, PAGE_SIZE-offset-len);
+                                       memset(kaddr+offset+len, 0,
+                                               PAGE_SIZE-offset-len);
+                       }
+                       err = copy_from_user(kaddr+offset, str, bytes_to_copy);
+                       if (err) {
+                               ret = -EFAULT;
+                               goto out;
                        }
-                       err = copy_from_user(kaddr + offset, str, bytes_to_copy);
-                       kunmap(page);
-
-                       if (err)
-                               return -EFAULT; 
 
                        pos += bytes_to_copy;
                        str += bytes_to_copy;
                        len -= bytes_to_copy;
                }
        }
-       return 0;
+       ret = 0;
+out:
+       if (kmapped_page)
+               kunmap(kmapped_page);
+       return ret;
 }
 
 /*