From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 929702A07 for ; Thu, 7 May 2015 08:58:29 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 06 May 2015 23:58:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,383,1427785200"; d="scan'208";a="567603578" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga003.jf.intel.com with ESMTP; 06 May 2015 23:58:24 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.224.2; Thu, 7 May 2015 14:54:14 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.107]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.69]) with mapi id 14.03.0224.002; Thu, 7 May 2015 14:54:12 +0800 From: "Xie, Huawei" To: Pavel Boldin , "dev@dpdk.org" Thread-Topic: [PATCH v5 4/5] vhost: eventfd_link: replace copy-pasted sys_close Thread-Index: AdCIkp+vzbvalRbaRUqdIVZZO2oQKw== 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> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v5 4/5] vhost: eventfd_link: replace copy-pasted sys_close 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: Thu, 07 May 2015 06:58:30 -0000 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=