From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Xie, Huawei" Subject: Re: [PATCH v5 4/5] vhost: eventfd_link: replace copy-pasted sys_close Date: Thu, 7 May 2015 06:54:12 +0000 Message-ID: References: <1427994080-10163-1-git-send-email-pboldin@mirantis.com> <1429184910-30186-1-git-send-email-pboldin@mirantis.com> <1429184910-30186-5-git-send-email-pboldin@mirantis.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To: Pavel Boldin , "dev-VfR2kkLFssw@public.gmane.org" Return-path: Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" On 4/16/2015 7:48 PM, Pavel Boldin wrote:=0A= > Replace copy-pasted `fget_from_files' -> `filp_close' with=0A= > a `sys_close' call.=0A= >=0A= > Signed-off-by: Pavel Boldin =0A= > ---=0A= > lib/librte_vhost/eventfd_link/eventfd_link.c | 49 +++++++---------------= ------=0A= > 1 file changed, 12 insertions(+), 37 deletions(-)=0A= >=0A= > diff --git a/lib/librte_vhost/eventfd_link/eventfd_link.c b/lib/librte_vh= ost/eventfd_link/eventfd_link.c=0A= > index 0a06594..9bc52a3 100644=0A= > --- a/lib/librte_vhost/eventfd_link/eventfd_link.c=0A= > +++ b/lib/librte_vhost/eventfd_link/eventfd_link.c=0A= > @@ -88,9 +88,8 @@ eventfd_link_ioctl_copy(unsigned long arg)=0A= > {=0A= > =0A= > + /* Closing the source_fd */=0A= > + ret =3D sys_close(eventfd_copy.source_fd);=0A= Pavel:=0A= Here we close the fd and re-install a new file on this fd later. =0A= sys_close does all cleanup.=0A= But, for instance, if we allocate new fd later, normally it will reuse=0A= the just freed fds by sys_close, is there issue here? =0A= =0A= > + if (ret)=0A= > goto out_task;=0A= > - }=0A= > -=0A= > - /*=0A= > - * Release the existing eventfd in the source process=0A= > - */=0A= > - spin_lock(&files->file_lock);=0A= > - fput(file);=0A= > - filp_close(file, files);=0A= > - fdt =3D files_fdtable(files);=0A= > - fdt->fd[eventfd_copy.source_fd] =3D NULL;=0A= > - spin_unlock(&files->file_lock);=0A= > -=0A= > - put_files_struct(files);=0A= > + ret =3D -ESTALE;=0A= > =0A= > /*=0A= > * Find the file struct associated with the target fd.=0A= > */=0A= > =0A= > - ret =3D -ESTALE;=0A= > - files =3D get_files_struct(task_target);=0A= > - if (files =3D=3D NULL) {=0A= > + target_files =3D get_files_struct(task_target);=0A= > + if (target_files =3D=3D NULL) {=0A= > pr_info("Failed to get target files struct\n");=0A= > goto out_task;=0A= > }=0A= > =0A= > ret =3D -EBADF;=0A= > - file =3D fget_from_files(files, eventfd_copy.target_fd);=0A= > - put_files_struct(files);=0A= > + target_file =3D fget_from_files(target_files, eventfd_copy.target_fd);= =0A= > + put_files_struct(target_files);=0A= > =0A= > - if (file =3D=3D NULL) {=0A= > + if (target_file =3D=3D NULL) {=0A= > pr_info("Failed to get fd %d from target\n",=0A= > eventfd_copy.target_fd);=0A= > goto out_task;=0A= > @@ -164,7 +139,7 @@ eventfd_link_ioctl_copy(unsigned long arg)=0A= > * file desciptor of the source process,=0A= > */=0A= > =0A= > - fd_install(eventfd_copy.source_fd, file);=0A= > + fd_install(eventfd_copy.source_fd, target_file);=0A= > ret =3D 0;=0A= > =0A= > out_task:=0A= =0A=