From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavel Boldin Subject: Re: [PATCH v5 1/4] vhost: eventfd_link: refactoring EVENTFD_COPY handler Date: Wed, 23 Sep 2015 23:25:50 +0300 Message-ID: References: <1429184910-30186-2-git-send-email-pboldin@mirantis.com> <1440787880-7079-1-git-send-email-pboldin@mirantis.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 To: "dev@dpdk.org" Return-path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by dpdk.org (Postfix) with ESMTP id BFB298E58 for ; Wed, 23 Sep 2015 22:25:50 +0200 (CEST) Received: by wicfx3 with SMTP id fx3so860935wic.1 for ; Wed, 23 Sep 2015 13:25:50 -0700 (PDT) In-Reply-To: <1440787880-7079-1-git-send-email-pboldin@mirantis.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Ping. On Fri, Aug 28, 2015 at 9:51 PM, Pavel Boldin wrote: > * 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 > --- > lib/librte_vhost/eventfd_link/eventfd_link.c | 188 > +++++++++++++++------------ > 1 file changed, 103 insertions(+), 85 deletions(-) > > diff --git a/lib/librte_vhost/eventfd_link/eventfd_link.c > b/lib/librte_vhost/eventfd_link/eventfd_link.c > index 62c45c8..5ba1068 100644 > --- a/lib/librte_vhost/eventfd_link/eventfd_link.c > +++ b/lib/librte_vhost/eventfd_link/eventfd_link.c > @@ -22,18 +22,11 @@ > * Intel Corporation > */ > > -#include > #include > #include > -#include > -#include > #include > -#include > -#include > -#include > -#include > -#include > #include > +#include > > #include "eventfd_link.h" > > @@ -65,9 +58,26 @@ 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 +85,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; > - } > - > - files = get_files_struct(current); > - if (files == NULL) { > - pr_debug("Failed to get files struct\n"); > - return -EFAULT; > - } > - > - 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); > + if (copy_from_user(&eventfd_copy, argp, sizeof(struct > eventfd_copy))) > + goto out; > + > + /* > + * Find the task struct for the target pid > + */ > + ret = -ESRCH; > + > + 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; > + } > + > + ret = -ESTALE; > + files = get_files_struct(current); > + if (files == NULL) { > + pr_info("Failed to get current files struct\n"); > + goto out_task; > + } > > - if (file == NULL) { > - pr_debug("Failed to get file from source 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); > - > - /* > - * 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; > - } > - > - 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(); > + ret = -EBADF; > + file = fget_from_files(files, eventfd_copy.source_fd); > + > + if (file == NULL) { > + pr_info("Failed to get fd %d from source\n", > + eventfd_copy.source_fd); > put_files_struct(files); > + 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); > + > + 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); > + > + if (file == NULL) { > + pr_info("Failed to get fd %d from target\n", > + eventfd_copy.target_fd); > + goto out_task; > + } > > - if (file == NULL) { > - pr_debug("Failed to get file from target pid\n"); > - return 0; > - } > + /* > + * Install the file struct from the target process into the > + * file desciptor of the source process, > + */ > > - /* > - * Install the file struct from the target process into the > - * file desciptor of the source process, > - */ > + fd_install(eventfd_copy.source_fd, file); > + ret = 0; > > - fd_install(eventfd_copy.source_fd, file); > +out_task: > + put_task_struct(task_target); > +out: > + return ret; > +} > > - return 0; > +static long > +eventfd_link_ioctl(struct file *f, unsigned int ioctl, unsigned long arg) > +{ > + long ret = -ENOIOCTLCMD; > > - default: > - return -ENOIOCTLCMD; > + switch (ioctl) { > + case EVENTFD_COPY: > + ret = eventfd_link_ioctl_copy(arg); > + break; > } > + > + return ret; > } > > static const struct file_operations eventfd_link_fops = { > -- > 1.9.1 > >