vhost: refactor eventfd copy handler
authorPavel Boldin <pboldin@mirantis.com>
Wed, 28 Oct 2015 18:33:47 +0000 (20:33 +0200)
committerThomas Monjalon <thomas.monjalon@6wind.com>
Fri, 30 Oct 2015 19:03:51 +0000 (20:03 +0100)
* Move ioctl `EVENTFD_COPY' code to a separate function
* Remove extra #includes
* Introduce function fget_from_files
* Fix ioctl return values

Signed-off-by: Pavel Boldin <pboldin@mirantis.com>
Acked-by: Huawei Xie <huawei.xie@intel.com>
lib/librte_vhost/eventfd_link/eventfd_link.c

index 62c45c8..7cbebd4 100644 (file)
  *   Intel Corporation
  */
 
-#include <linux/eventfd.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/rcupdate.h>
 #include <linux/file.h>
-#include <linux/slab.h>
-#include <linux/fs.h>
-#include <linux/mmu_context.h>
-#include <linux/sched.h>
-#include <asm/mmu_context.h>
 #include <linux/fdtable.h>
+#include <linux/syscalls.h>
 
 #include "eventfd_link.h"
 
@@ -65,9 +58,27 @@ put_files_struct(struct files_struct *files)
                BUG();
 }
 
+static struct file *
+fget_from_files(struct files_struct *files, unsigned fd)
+{
+       struct file *file;
+
+       rcu_read_lock();
+       file = fcheck_files(files, fd);
+       if (file) {
+               if (file->f_mode & FMODE_PATH ||
+                       !atomic_long_inc_not_zero(&file->f_count)) {
+
+                       file = NULL;
+               }
+       }
+       rcu_read_unlock();
+
+       return file;
+}
 
 static long
-eventfd_link_ioctl(struct file *f, unsigned int ioctl, unsigned long arg)
+eventfd_link_ioctl_copy(unsigned long arg)
 {
        void __user *argp = (void __user *) arg;
        struct task_struct *task_target = NULL;
@@ -75,91 +86,99 @@ eventfd_link_ioctl(struct file *f, unsigned int ioctl, unsigned long arg)
        struct files_struct *files;
        struct fdtable *fdt;
        struct eventfd_copy eventfd_copy;
+       long ret = -EFAULT;
 
-       switch (ioctl) {
-       case EVENTFD_COPY:
-               if (copy_from_user(&eventfd_copy, argp,
-                       sizeof(struct eventfd_copy)))
-                       return -EFAULT;
-
-               /*
-                * Find the task struct for the target pid
-                */
-               task_target =
-                       pid_task(find_vpid(eventfd_copy.target_pid), PIDTYPE_PID);
-               if (task_target == NULL) {
-                       pr_debug("Failed to get mem ctx for target pid\n");
-                       return -EFAULT;
-               }
+       if (copy_from_user(&eventfd_copy, argp, sizeof(struct eventfd_copy)))
+               goto out;
 
-               files = get_files_struct(current);
-               if (files == NULL) {
-                       pr_debug("Failed to get files struct\n");
-                       return -EFAULT;
-               }
+       /*
+        * Find the task struct for the target pid
+        */
+       ret = -ESRCH;
 
-               rcu_read_lock();
-               file = fcheck_files(files, eventfd_copy.source_fd);
-               if (file) {
-                       if (file->f_mode & FMODE_PATH ||
-                               !atomic_long_inc_not_zero(&file->f_count))
-                               file = NULL;
-               }
-               rcu_read_unlock();
-               put_files_struct(files);
+       task_target =
+               get_pid_task(find_vpid(eventfd_copy.target_pid), PIDTYPE_PID);
+       if (task_target == NULL) {
+               pr_info("Unable to find pid %d\n", eventfd_copy.target_pid);
+               goto out;
+       }
 
-               if (file == NULL) {
-                       pr_debug("Failed to get file from source pid\n");
-                       return 0;
-               }
+       ret = -ESTALE;
+       files = get_files_struct(current);
+       if (files == NULL) {
+               pr_info("Failed to get current files struct\n");
+               goto out_task;
+       }
 
-               /*
-                * Release the existing eventfd in the source process
-                */
-               spin_lock(&files->file_lock);
-               fput(file);
-               filp_close(file, files);
-               fdt = files_fdtable(files);
-               fdt->fd[eventfd_copy.source_fd] = NULL;
-               spin_unlock(&files->file_lock);
-
-               /*
-                * Find the file struct associated with the target fd.
-                */
-
-               files = get_files_struct(task_target);
-               if (files == NULL) {
-                       pr_debug("Failed to get files struct\n");
-                       return -EFAULT;
-               }
+       ret = -EBADF;
+       file = fget_from_files(files, eventfd_copy.source_fd);
 
-               rcu_read_lock();
-               file = fcheck_files(files, eventfd_copy.target_fd);
-               if (file) {
-                       if (file->f_mode & FMODE_PATH ||
-                               !atomic_long_inc_not_zero(&file->f_count))
-                                       file = NULL;
-               }
-               rcu_read_unlock();
+       if (file == NULL) {
+               pr_info("Failed to get fd %d from source\n",
+                       eventfd_copy.source_fd);
                put_files_struct(files);
+               goto out_task;
+       }
 
-               if (file == NULL) {
-                       pr_debug("Failed to get file from target pid\n");
-                       return 0;
-               }
+       /*
+        * Release the existing eventfd in the source process
+        */
+       spin_lock(&files->file_lock);
+       fput(file);
+       filp_close(file, files);
+       fdt = files_fdtable(files);
+       fdt->fd[eventfd_copy.source_fd] = NULL;
+       spin_unlock(&files->file_lock);
+
+       put_files_struct(files);
+
+       /*
+        * Find the file struct associated with the target fd.
+        */
+
+       ret = -ESTALE;
+       files = get_files_struct(task_target);
+       if (files == NULL) {
+               pr_info("Failed to get target files struct\n");
+               goto out_task;
+       }
+
+       ret = -EBADF;
+       file = fget_from_files(files, eventfd_copy.target_fd);
+       put_files_struct(files);
 
-               /*
-                * Install the file struct from the target process into the
-                * file desciptor of the source process,
-                */
+       if (file == NULL) {
+               pr_info("Failed to get fd %d from target\n",
+                       eventfd_copy.target_fd);
+               goto out_task;
+       }
 
-               fd_install(eventfd_copy.source_fd, file);
+       /*
+        * Install the file struct from the target process into the
+        * file desciptor of the source process,
+        */
 
-               return 0;
+       fd_install(eventfd_copy.source_fd, file);
+       ret = 0;
 
-       default:
-               return -ENOIOCTLCMD;
+out_task:
+       put_task_struct(task_target);
+out:
+       return ret;
+}
+
+static long
+eventfd_link_ioctl(struct file *f, unsigned int ioctl, unsigned long arg)
+{
+       long ret = -ENOIOCTLCMD;
+
+       switch (ioctl) {
+       case EVENTFD_COPY:
+               ret = eventfd_link_ioctl_copy(arg);
+               break;
        }
+
+       return ret;
 }
 
 static const struct file_operations eventfd_link_fops = {