From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mirantis.com; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=ViO4ytsgeC2flgrLisDqkzoYEUyUY2oZZjzS8OpaVl4=; b=H6B5LtQdweBkOfQDSCxGbIpBh5Q0VSmirWBqxhfTzQYFvF6YCv9+qo0ghG6iRsV6Wy 7FpIQUualHHaqbH/AKfDSna7ZEzbcD8qoqoczfGe3lM1YC0x2XxIX/YvForc/QJ3WI6n 7yZ81KX3W6v3XUP2mO5/48l8vVFGBRsZgcPCc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ViO4ytsgeC2flgrLisDqkzoYEUyUY2oZZjzS8OpaVl4=; b=VIHa7PqNM2HvDI59aqqS570FJPF1kOxoATnLID8O1HPcC+6HH+TRTaS3LljKnLETPQ 8eRkOVMcZQPwttuOYTdDHYX5ibIaD/S3SG9SB2/ix6cEaSQmNXkl3f+oRKuqD+hCkO4P EtM2nFzIWAosDDUjvG3YlsVOht2juO+gyEIc0eKSNtLLpkby1kjXJoahLqa7HTo3kNZm hQwdm94NNBteVAlGdun2SNeX7JdbJ9+AIfh+pQmjhInD+sqA/sYQBxnSmSfoKWh0qus8 W8AFDVl2XmkiEHXRqDiiQ1QNj9g43hNQQfS0vCx2ogUY28sGr1t27M4xiVoLiceal1h0 dmfg== X-Gm-Message-State: ALoCoQl6Ky/3CTZhP0ZsnivTzp+hQmklUJOP5kranz6AVuE9EHHCCOIpfHKHBCzQVU5ydkJ2NxOt MIME-Version: 1.0 X-Received: by 10.194.85.12 with SMTP id d12mr13012809wjz.53.1443039950522; Wed, 23 Sep 2015 13:25:50 -0700 (PDT) Received: by 10.194.127.199 with HTTP; Wed, 23 Sep 2015 13:25:50 -0700 (PDT) In-Reply-To: <1440787880-7079-1-git-send-email-pboldin@mirantis.com> References: <1429184910-30186-2-git-send-email-pboldin@mirantis.com> <1440787880-7079-1-git-send-email-pboldin@mirantis.com> Date: Wed, 23 Sep 2015 23:25:50 +0300 Message-ID: From: Pavel Boldin To: "dev@dpdk.org" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v5 1/4] vhost: eventfd_link: refactoring EVENTFD_COPY handler X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Sep 2015 20:25:51 -0000 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 > >