From owner-freebsd-fs@freebsd.org Thu Jul 9 10:25:39 2015 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6787C9970F6 for ; Thu, 9 Jul 2015 10:25:39 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E767214D0; Thu, 9 Jul 2015 10:25:38 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t69APX3J013244 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 9 Jul 2015 13:25:33 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t69APX3J013244 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t69APXxD013243; Thu, 9 Jul 2015 13:25:33 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 9 Jul 2015 13:25:33 +0300 From: Konstantin Belousov To: Mateusz Guzik Cc: rwatson@FreeBSD.org, freebsd-fs@freebsd.org, Mateusz Guzik Subject: Re: [PATCH 3/4] vfs: simplify error handling in namei Message-ID: <20150709102533.GO2080@kib.kiev.ua> References: <20150707085857.GZ2080@kib.kiev.ua> <1436393231-5831-1-git-send-email-mjguzik@gmail.com> <1436393231-5831-4-git-send-email-mjguzik@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436393231-5831-4-git-send-email-mjguzik@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jul 2015 10:25:39 -0000 On Thu, Jul 09, 2015 at 12:07:10AM +0200, Mateusz Guzik wrote: > From: Mateusz Guzik > > The logic is reorganised so that there is one exit point prior to the > lookup loop. This is an intermediate step to making audit logging > functions use found vnode instead of translating ni_dirfd on their own. > > ni_startdir validation is removed. The only in-tree consumer is nfs > which already makes sure it is a directory. > --- > sys/kern/vfs_lookup.c | 50 +++++++++++++++++++++----------------------------- > 1 file changed, 21 insertions(+), 29 deletions(-) > > diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c > index e434464..d48fcff 100644 > --- a/sys/kern/vfs_lookup.c > +++ b/sys/kern/vfs_lookup.c > @@ -158,7 +158,7 @@ namei(struct nameidata *ndp) > struct vnode *dp; /* the directory we are searching */ > struct iovec aiov; /* uio for reading symbolic links */ > struct uio auio; > - int error, linklen; > + int error, linklen, startdir_used; > struct componentname *cnp = &ndp->ni_cnd; > struct thread *td = cnp->cn_thread; > struct proc *p = td->td_proc; > @@ -169,6 +169,9 @@ namei(struct nameidata *ndp) > ("namei: nameiop contaminated with flags")); > KASSERT((cnp->cn_flags & OPMASK) == 0, > ("namei: flags contaminated with nameiops")); > + if (ndp->ni_startdir != NULL) > + MPASS(ndp->ni_startdir->v_type == VDIR || > + ndp->ni_startdir->v_type == VBAD); Write this as MPASS(ndp->ni_startdir == NULL || ... == VDIR || ... == VBAD); ? I think that the two previous patches are self-contained ? Please commit them, after that I think review of this patch can be finished. > if (!lookup_shared) > cnp->cn_flags &= ~LOCKSHARED; > fdp = p->p_fd; > @@ -242,23 +245,19 @@ namei(struct nameidata *ndp) > if (cnp->cn_flags & AUDITVNODE2) > AUDIT_ARG_UPATH2(td, ndp->ni_dirfd, cnp->cn_pnbuf); > > + startdir_used = 0; > dp = NULL; > cnp->cn_nameptr = cnp->cn_pnbuf; > if (cnp->cn_pnbuf[0] == '/') { > error = namei_handle_root(ndp, &dp); > - FILEDESC_SUNLOCK(fdp); > - if (error != 0) { > - vrele(ndp->ni_rootdir); > - if (ndp->ni_startdir != NULL) > - vrele(ndp->ni_startdir); > - namei_cleanup_cnp(cnp); > - return (error); > - } > } else { > if (ndp->ni_startdir != NULL) { > dp = ndp->ni_startdir; > - error = 0; > - } else if (ndp->ni_dirfd != AT_FDCWD) { > + startdir_used = 1; > + } else if (ndp->ni_dirfd == AT_FDCWD) { > + dp = fdp->fd_cdir; > + VREF(dp); > + } else { > cap_rights_t rights; > > rights = ndp->ni_rightsneeded; > @@ -285,25 +284,18 @@ namei(struct nameidata *ndp) > } > #endif > } > - if (error != 0 || dp != NULL) { > - FILEDESC_SUNLOCK(fdp); > - if (error == 0 && dp->v_type != VDIR) { > - vrele(dp); > - error = ENOTDIR; > - } > - } > - if (error) { > - vrele(ndp->ni_rootdir); > - namei_cleanup_cnp(cnp); > - return (error); > - } > + if (error == 0 && dp->v_type != VDIR) > + error = ENOTDIR; > } > - if (dp == NULL) { > - dp = fdp->fd_cdir; > - VREF(dp); > - FILEDESC_SUNLOCK(fdp); > - if (ndp->ni_startdir != NULL) > - vrele(ndp->ni_startdir); > + FILEDESC_SUNLOCK(fdp); > + if (ndp->ni_startdir != NULL && !startdir_used) > + vrele(ndp->ni_startdir); > + if (error != 0) { > + if (dp != NULL) > + vrele(dp); > + vrele(ndp->ni_rootdir); > + namei_cleanup_cnp(cnp); > + return (error); > } > SDT_PROBE(vfs, namei, lookup, entry, dp, cnp->cn_pnbuf, > cnp->cn_flags, 0, 0); > -- > 2.4.5