From owner-p4-projects@FreeBSD.ORG Mon Jun 7 15:33:01 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id C45521065670; Mon, 7 Jun 2010 15:33:01 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 88C81106564A for ; Mon, 7 Jun 2010 15:33:01 +0000 (UTC) (envelope-from gpf@FreeBSD.org) Received: from repoman.freebsd.org (unknown [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 75D568FC0A for ; Mon, 7 Jun 2010 15:33:01 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id o57FX18u003506 for ; Mon, 7 Jun 2010 15:33:01 GMT (envelope-from gpf@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o57FX1Rk003504 for perforce@freebsd.org; Mon, 7 Jun 2010 15:33:01 GMT (envelope-from gpf@FreeBSD.org) Date: Mon, 7 Jun 2010 15:33:01 GMT Message-Id: <201006071533.o57FX1Rk003504@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gpf@FreeBSD.org using -f From: Efstratios Karatzas To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 179289 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Jun 2010 15:33:02 -0000 http://p4web.freebsd.org/@@179289?ac=10 Change 179289 by gpf@gpf_desktop on 2010/06/07 15:32:53 - make vn_fullpath_nocache() friendly to non-mp safe fs - removed some useless locks or references - some code refactoring, edited comments etc - fixed a tiny bug in vn_fullpath_nocache() Affected files ... .. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/kern/vfs_cache.c#3 edit .. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/ufs/ffs/ffs_vnops.c#7 edit Differences ... ==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/kern/vfs_cache.c#3 (text+ko) ==== @@ -1227,43 +1227,53 @@ return (0); } +/* + * XXXgpf: should relocate them someplace else + * I just dont know where:S + */ +#define PARENTHINT 0x0001 +#define EXHAUSTSEARCH 0x0002 +#define WANTNAME 0x0004 + /* - * vn_fullpath_nocache + * vn_fullpath_nocache(9) * * Retrieve the full filesystem path that corresponds to a vnode without use of the * name cache. * - A directory hint (UFS file_id of the directory that contains the vnode) may be * supplied to facilitate the search if our target is not a directory itself. * - flags should be set to PARENT_HINT, if the directory hint is supplied - * and to EXHAUSTIVE_SEARCH, if we are willing to go intro great trouble to get this path. + * and to EXHAUSTIVE_SEARCH, if we are willing to search the entire filesystem to acquire a path. * * Locks: no locks required. * - * Author's note: This only works for UFS filesystems (for now). - * Oh, also EXHAUSTIVE_SEARCH will kernel panic :-D + * Authors Note: freepath should be set to NULL before calling this KPI. Upon returning from the KPI, + * the caller should check if freepath is non-NULL, and if so, invoke free(9) with a pool type of M_TEMP. */ int vn_fullpath_nocache(struct vnode *vp, char **fullpath, char **freepath, uint64_t directory_hint, char flags) { + char fname[MNAMELEN]; struct vnode *dvp, *upper_dvp; struct mount *mp; struct thread * td; - char *buf, *pch; - char fname[MNAMELEN]; + char *buf, *pch; int error, buflen, vfslocked, fnamelen; KASSERT(vp != NULL, ("vn_fullpath_nocache: null vp")); dvp = NULL; buf = NULL; - *freepath = NULL; + *freepath = NULL; + VREF(vp); + /* doesn't really make sense to continue without this flag set */ + flags |= WANTNAME; + /* XXXgpf: do we really need this? */ if (vp->v_type == VBAD) { error = ENOENT; goto out; - } - - vref(vp); + } error = 0; td = curthread; mp = vp->v_mount; @@ -1286,12 +1296,18 @@ goto out; } - /* we have found a parent directory and a name for our vnode, save the name */ + /* + * we have found a parent directory and a name for our vnode, + * save the name at the end of buf + */ pch = buf + buflen - strlen(fname); if (pch < buf) { error = EOVERFLOW; - if (dvp != NULL) + if (dvp != NULL) { + vfslocked = VFS_LOCK_GIANT(dvp->v_mount); vput(dvp); + VFS_UNLOCK_GIANT(vfslocked); + } goto out; } strcpy(pch, fname); @@ -1301,7 +1317,7 @@ /* if our target is a dir, do the initial preparation */ else { dvp = vp; - vref(dvp); + VREF(dvp); vn_lock(dvp, LK_SHARED); } @@ -1313,54 +1329,58 @@ /* * If we've found a vnode that is the root of a filesystem * Use the path that the filesystem was mounted on to complete our fullpath - * - * XXXgpf: how safe is it to use the path from the statistics of a mounted fs? - * the size of the f_mntonname field seems kinda small :-S */ if ((dvp->v_vflag & VV_ROOT) != 0) { char *pch, *fs_path; int fs_path_len; - - vfslocked = VFS_LOCK_GIANT(dvp->v_mount); + MNT_REF(dvp->v_mount); *fullpath = buf + buflen; - fs_path = dvp->v_mount->mnt_stat.f_mntonname; fs_path_len = strlen(fs_path); - + if (buflen - fs_path_len - 1 < 0) { + vfslocked = VFS_LOCK_GIANT(dvp->v_mount); vput(dvp); + VFS_UNLOCK_GIANT(vfslocked); error = EOVERFLOW; - VFS_UNLOCK_GIANT(vfslocked); + MNT_REL(dvp->v_mount); goto out; } - + pch = buf + buflen - fs_path_len; memcpy(pch, fs_path, fs_path_len); buflen -= fs_path_len; - - VFS_UNLOCK_GIANT(vfslocked); - + + MNT_REL(dvp->v_mount); break; } - + + vfslocked = VFS_LOCK_GIANT(dvp->v_mount); error = VOP_VPTOCNP(dvp, &upper_dvp, td->td_ucred, buf, &buflen); - if (error) { - uprintf("VOP_VPTOCNP failure %d\n", error); + VFS_UNLOCK_GIANT(vfslocked); + if (error) + break; + if (buflen <= 0) { + error = EOVERFLOW; break; } - - buf[--buflen] = '/'; - if (dvp != NULL) + + buf[--buflen] = '/'; + if (dvp != NULL) { + vfslocked = VFS_LOCK_GIANT(dvp->v_mount); vput(dvp); - + VFS_UNLOCK_GIANT(vfslocked); + } vdrop(upper_dvp); dvp = upper_dvp; vn_lock(dvp, LK_SHARED); - vref(dvp); + VREF(dvp); } /* while */ - vput(dvp); + vfslocked = VFS_LOCK_GIANT(dvp->v_mount); + vput(dvp); + VFS_UNLOCK_GIANT(vfslocked); *fullpath = buf + buflen; *freepath = buf; @@ -1369,8 +1389,10 @@ *freepath = NULL; if (buf != NULL) free(buf, M_TEMP); - } + } + vfslocked = VFS_LOCK_GIANT(vp->v_mount); vrele(vp); - + VFS_UNLOCK_GIANT(vfslocked); + return error; } ==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/ufs/ffs/ffs_vnops.c#7 (text+ko) ==== @@ -1806,7 +1806,7 @@ * * find 'a' name that is used to reference vp inside some parent directory. * flags should be set to WANTNAME if the filename should be copied to - * the supplied buffer. + * the supplied buffer, in which case the supplied buffer should be != NULL. * * flags should be set to EXHAUSTSEARCH if want to perform a depth first search * on the filesystem that contains vp. In this case, tmpdvp should be the root vnode @@ -1951,7 +1951,9 @@ * int flags, char *buf, int *buflen); * * Find a parent directory -dvp- with vp as a child. The parent hint is used to - * facilitate the search. + * facilitate the search. On success, the name used to reference the child -vp- + * will be copied to buf. buflen should contain the lenght of buf on entry. On + * success, it will contain the remaining length of the buffer. * * Flags should be set to: * - PARENTHIT: if a hint ino_t of a directory is supplied to facilitate the search @@ -1989,8 +1991,6 @@ if (flags & WANTNAME) KASSERT(ap->a_buf != NULL, ("VOP_GEPARENT: null buffer")); - MNT_REF(mp); - /* XXXgpf:is this check necessary? */ if (vp->v_type == VBAD) { error = ENOENT; @@ -2040,14 +2040,10 @@ } out: - if (error == 0 && dvp != NULL) { + if (error == 0 && dvp != NULL) *(ap->a_vpp) = dvp; - } - else { + else *(ap->a_vpp) = NULL; - } - - MNT_REL(mp); return error; }