ANDROID: goldfish_pipe: bugfixes and performance improvements.
authorYurii Zubrytskyi <zyy@google.com>
Wed, 4 May 2016 20:05:38 +0000 (13:05 -0700)
committerAmit Pundir <amit.pundir@linaro.org>
Thu, 1 Dec 2016 09:48:44 +0000 (15:18 +0530)
Combine following patches from android-goldfish-3.18 branch:

c0f015a [pipe] Fix the pipe driver for x64 platform + correct pages count
48e6bf5 [pipe] Use get_use_pages_fast() which is possibly faster
fb20f13 [goldfish] More pages in goldfish pipe
f180e6d goldfish_pipe: Return from read_write on signal and EIO
3dec3b7 [pipe] Fix a minor leak in setup_access_params_addr()

Change-Id: I1041fd65d7faaec123e6cedd3dbbc5a2fbb86c4d

drivers/platform/goldfish/goldfish_pipe.c

index 3215a33cf4fe516e7af76215945ecb3be5ccfcd8..55929ed58ea3adb6c6d599088383272ebb1bb88f 100644 (file)
 #define PIPE_WAKE_READ         (1 << 1)  /* pipe can now be read from */
 #define PIPE_WAKE_WRITE        (1 << 2)  /* pipe can now be written to */
 
+#define MAX_PAGES_TO_GRAB 32
+
+#define DEBUG 0
+
+#if DEBUG
+#define DPRINT(...) { printk(KERN_ERR __VA_ARGS__); }
+#else
+#define DPRINT(...)
+#endif
+
 struct access_params {
        unsigned long channel;
        u32 size;
@@ -231,8 +241,10 @@ static int setup_access_params_addr(struct platform_device *pdev,
        if (valid_batchbuffer_addr(dev, aps)) {
                dev->aps = aps;
                return 0;
-       } else
+       } else {
+               devm_kfree(&pdev->dev, aps);
                return -1;
+       }
 }
 
 /* A value that will not be set by qemu emulator */
@@ -269,6 +281,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
        struct goldfish_pipe *pipe = filp->private_data;
        struct goldfish_pipe_dev *dev = pipe->dev;
        unsigned long address, address_end;
+       struct page* pages[MAX_PAGES_TO_GRAB] = {};
        int count = 0, ret = -EINVAL;
 
        /* If the emulator already closed the pipe, no need to go further */
@@ -292,45 +305,59 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
        address_end = address + bufflen;
 
        while (address < address_end) {
-               unsigned long  page_end = (address & PAGE_MASK) + PAGE_SIZE;
-               unsigned long  next     = page_end < address_end ? page_end
-                                                                : address_end;
-               unsigned long  avail    = next - address;
-               int status, wakeBit;
-
-               struct page *page;
-
-               /* Either vaddr or paddr depending on the device version */
-               unsigned long xaddr;
+               unsigned long page_end = (address & PAGE_MASK) + PAGE_SIZE;
+               unsigned long next, avail;
+               int status, wakeBit, page_i, num_contiguous_pages;
+               long first_page, last_page, requested_pages;
+               unsigned long xaddr, xaddr_prev, xaddr_i;
 
                /*
-                * We grab the pages on a page-by-page basis in case user
-                * space gives us a potentially huge buffer but the read only
-                * returns a small amount, then there's no need to pin that
-                * much memory to the process.
+                * Attempt to grab multiple physically contiguous pages.
                 */
-               down_read(&current->mm->mmap_sem);
-               ret = get_user_pages(current, current->mm, address, 1,
-                                    !is_write, 0, &page, NULL);
-               up_read(&current->mm->mmap_sem);
-               if (ret < 0)
+               first_page = address & PAGE_MASK;
+               last_page = (address_end - 1) & PAGE_MASK;
+               requested_pages = ((last_page - first_page) >> PAGE_SHIFT) + 1;
+               if (requested_pages > MAX_PAGES_TO_GRAB) {
+                       requested_pages = MAX_PAGES_TO_GRAB;
+               }
+               ret = get_user_pages_fast(first_page, requested_pages,
+                               !is_write, pages);
+
+               DPRINT("%s: requested pages: %d %d\n", __FUNCTION__, ret, requested_pages);
+               if (ret == 0) {
+                       DPRINT("%s: error: (requested pages == 0) (wanted %d)\n",
+                                       __FUNCTION__, requested_pages);
+                       return ret;
+               }
+               if (ret < 0) {
+                       DPRINT("%s: (requested pages < 0) %d \n",
+                                       __FUNCTION__, requested_pages);
                        return ret;
+               }
 
-               if (dev->version) {
-                       /* Device version 1 or newer (qemu-android) expects the
-                        * physical address. */
-                       xaddr = page_to_phys(page) | (address & ~PAGE_MASK);
-               } else {
-                       /* Device version 0 (classic emulator) expects the
-                        * virtual address. */
-                       xaddr = address;
+               xaddr = page_to_phys(pages[0]) | (address & ~PAGE_MASK);
+               xaddr_prev = xaddr;
+               num_contiguous_pages = ret == 0 ? 0 : 1;
+               for (page_i = 1; page_i < ret; page_i++) {
+                       xaddr_i = page_to_phys(pages[page_i]) | (address & ~PAGE_MASK);
+                       if (xaddr_i == xaddr_prev + PAGE_SIZE) {
+                               page_end += PAGE_SIZE;
+                               xaddr_prev = xaddr_i;
+                               num_contiguous_pages++;
+                       } else {
+                               DPRINT("%s: discontinuous page boundary: %d pages instead\n",
+                                               __FUNCTION__, page_i);
+                               break;
+                       }
                }
+               next = page_end < address_end ? page_end : address_end;
+               avail = next - address;
 
                /* Now, try to transfer the bytes in the current page */
                spin_lock_irqsave(&dev->lock, irq_flags);
                if (access_with_param(dev,
-                                     is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
-                                     xaddr, avail, pipe, &status)) {
+                                       is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
+                                       xaddr, avail, pipe, &status)) {
                        gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
                                     dev->base + PIPE_REG_CHANNEL_HIGH);
                        writel(avail, dev->base + PIPE_REG_SIZE);
@@ -343,9 +370,13 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
                }
                spin_unlock_irqrestore(&dev->lock, irq_flags);
 
-               if (status > 0 && !is_write)
-                       set_page_dirty(page);
-               put_page(page);
+               for (page_i = 0; page_i < ret; page_i++) {
+                       if (status > 0 && !is_write &&
+                               page_i < num_contiguous_pages) {
+                               set_page_dirty(pages[page_i]);
+                       }
+                       put_page(pages[page_i]);
+               }
 
                if (status > 0) { /* Correct transfer */
                        count += status;
@@ -367,7 +398,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
                         */
                        if (status != PIPE_ERROR_AGAIN)
                                pr_info_ratelimited("goldfish_pipe: backend returned error %d on %s\n",
-                                           status, is_write ? "write" : "read");
+                                               status, is_write ? "write" : "read");
                        ret = 0;
                        break;
                }
@@ -377,7 +408,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
                 * non-blocking mode, just return the error code.
                 */
                if (status != PIPE_ERROR_AGAIN ||
-                       (filp->f_flags & O_NONBLOCK) != 0) {
+                               (filp->f_flags & O_NONBLOCK) != 0) {
                        ret = goldfish_pipe_error_convert(status);
                        break;
                }
@@ -391,7 +422,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 
                /* Tell the emulator we're going to wait for a wake event */
                goldfish_cmd(pipe,
-                            is_write ? CMD_WAKE_ON_WRITE : CMD_WAKE_ON_READ);
+                               is_write ? CMD_WAKE_ON_WRITE : CMD_WAKE_ON_READ);
 
                /* Unlock the pipe, then wait for the wake signal */
                mutex_unlock(&pipe->lock);
@@ -399,15 +430,11 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
                while (test_bit(wakeBit, &pipe->flags)) {
                        if (wait_event_interruptible(
                                        pipe->wake_queue,
-                                       !test_bit(wakeBit, &pipe->flags))) {
-                               ret = -ERESTARTSYS;
-                               break;
-                       }
+                                       !test_bit(wakeBit, &pipe->flags)))
+                               return -ERESTARTSYS;
 
-                       if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) {
-                               ret = -EIO;
-                               break;
-                       }
+                       if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags))
+                               return -EIO;
                }
 
                /* Try to re-acquire the lock */
@@ -543,6 +570,8 @@ static int goldfish_pipe_open(struct inode *inode, struct file *file)
 
        pipe->dev = dev;
        mutex_init(&pipe->lock);
+       DPRINT("%s: call. pipe_dev pipe_dev=0x%lx new_pipe_addr=0x%lx file=0x%lx\n", __FUNCTION__, pipe_dev, pipe, file);
+       // spin lock init, write head of list, i guess
        init_waitqueue_head(&pipe->wake_queue);
 
        /*
@@ -565,6 +594,7 @@ static int goldfish_pipe_release(struct inode *inode, struct file *filp)
 {
        struct goldfish_pipe *pipe = filp->private_data;
 
+       DPRINT("%s: call. pipe=0x%lx file=0x%lx\n", __FUNCTION__, pipe, filp);
        /* The guest is closing the channel, so tell the emulator right now */
        goldfish_cmd(pipe, CMD_CLOSE);
        kfree(pipe);
@@ -589,6 +619,7 @@ static struct miscdevice goldfish_pipe_device = {
 
 static int goldfish_pipe_probe(struct platform_device *pdev)
 {
+       DPRINT("%s: call. platform_device=0x%lx\n", __FUNCTION__, pdev);
        int err;
        struct resource *r;
        struct goldfish_pipe_dev *dev = pipe_dev;