Date: Thu, 9 Jul 2015 13:25:33 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Mateusz Guzik <mjguzik@gmail.com> Cc: rwatson@FreeBSD.org, freebsd-fs@freebsd.org, Mateusz Guzik <mjg@freebsd.org> Subject: Re: [PATCH 3/4] vfs: simplify error handling in namei Message-ID: <20150709102533.GO2080@kib.kiev.ua> In-Reply-To: <1436393231-5831-4-git-send-email-mjguzik@gmail.com> References: <20150707085857.GZ2080@kib.kiev.ua> <1436393231-5831-1-git-send-email-mjguzik@gmail.com> <1436393231-5831-4-git-send-email-mjguzik@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Jul 09, 2015 at 12:07:10AM +0200, Mateusz Guzik wrote:
> From: Mateusz Guzik <mjg@freebsd.org>
>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150709102533.GO2080>