From owner-svn-src-all@FreeBSD.ORG Sun May 16 07:46:04 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 691F2106564A; Sun, 16 May 2010 07:46:04 +0000 (UTC) (envelope-from mm@FreeBSD.org) Received: from svn.freebsd.org (unknown [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 56FE68FC14; Sun, 16 May 2010 07:46:04 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o4G7k48g035792; Sun, 16 May 2010 07:46:04 GMT (envelope-from mm@svn.freebsd.org) Received: (from mm@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o4G7k3Es035787; Sun, 16 May 2010 07:46:03 GMT (envelope-from mm@svn.freebsd.org) Message-Id: <201005160746.o4G7k3Es035787@svn.freebsd.org> From: Martin Matuska Date: Sun, 16 May 2010 07:46:03 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r208131 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 May 2010 07:46:04 -0000 Author: mm Date: Sun May 16 07:46:03 2010 New Revision: 208131 URL: http://svn.freebsd.org/changeset/base/208131 Log: Fix deadlock between zfs_dirent_lock and zfs_rmdir OpenSolaris onnv revision: 11321:506b7043a14c Approved by: pjd, delphij (mentor) Obtained from: OpenSolaris (Bug ID 6847615) MFC after: 3 days Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_dir.h head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_dir.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_dir.h Sun May 16 07:16:28 2010 (r208130) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_dir.h Sun May 16 07:46:03 2010 (r208131) @@ -44,6 +44,7 @@ extern "C" { #define ZRENAMING 0x0010 /* znode is being renamed */ #define ZCILOOK 0x0020 /* case-insensitive lookup requested */ #define ZCIEXACT 0x0040 /* c-i requires c-s match (rename) */ +#define ZHAVELOCK 0x0080 /* z_name_lock is already held */ /* mknode flags */ #define IS_ROOT_NODE 0x01 /* create a root node */ Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h Sun May 16 07:16:28 2010 (r208130) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h Sun May 16 07:46:03 2010 (r208131) @@ -174,6 +174,7 @@ typedef struct znode_phys { typedef struct zfs_dirlock { char *dl_name; /* directory entry being locked */ uint32_t dl_sharecnt; /* 0 if exclusive, > 0 if shared */ + uint8_t dl_namelock; /* 1 if z_name_lock is NOT held */ uint16_t dl_namesize; /* set if dl_name was allocated */ kcondvar_t dl_cv; /* wait for entry to be unlocked */ struct znode *dl_dzp; /* directory znode */ Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c Sun May 16 07:16:28 2010 (r208130) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c Sun May 16 07:46:03 2010 (r208131) @@ -114,6 +114,8 @@ zfs_match_find(zfsvfs_t *zfsvfs, znode_t * ZCIEXACT: On a purely case-insensitive file system, * this lookup should be case-sensitive. * ZRENAMING: we are locking for renaming, force narrow locks + * ZHAVELOCK: Don't grab the z_name_lock for this call. The + * current thread already holds it. * * Output arguments: * zpp - pointer to the znode for the entry (NULL if there isn't one) @@ -208,13 +210,20 @@ zfs_dirent_lock(zfs_dirlock_t **dlpp, zn /* * Wait until there are no locks on this name. + * + * Don't grab the the lock if it is already held. However, cannot + * have both ZSHARED and ZHAVELOCK together. */ - rw_enter(&dzp->z_name_lock, RW_READER); + ASSERT(!(flag & ZSHARED) || !(flag & ZHAVELOCK)); + if (!(flag & ZHAVELOCK)) + rw_enter(&dzp->z_name_lock, RW_READER); + mutex_enter(&dzp->z_lock); for (;;) { if (dzp->z_unlinked) { mutex_exit(&dzp->z_lock); - rw_exit(&dzp->z_name_lock); + if (!(flag & ZHAVELOCK)) + rw_exit(&dzp->z_name_lock); return (ENOENT); } for (dl = dzp->z_dirlocks; dl != NULL; dl = dl->dl_next) { @@ -224,7 +233,8 @@ zfs_dirent_lock(zfs_dirlock_t **dlpp, zn } if (error != 0) { mutex_exit(&dzp->z_lock); - rw_exit(&dzp->z_name_lock); + if (!(flag & ZHAVELOCK)) + rw_exit(&dzp->z_name_lock); return (ENOENT); } if (dl == NULL) { @@ -235,6 +245,7 @@ zfs_dirent_lock(zfs_dirlock_t **dlpp, zn cv_init(&dl->dl_cv, NULL, CV_DEFAULT, NULL); dl->dl_name = name; dl->dl_sharecnt = 0; + dl->dl_namelock = 0; dl->dl_namesize = 0; dl->dl_dzp = dzp; dl->dl_next = dzp->z_dirlocks; @@ -246,6 +257,12 @@ zfs_dirent_lock(zfs_dirlock_t **dlpp, zn cv_wait(&dl->dl_cv, &dzp->z_lock); } + /* + * If the z_name_lock was NOT held for this dirlock record it. + */ + if (flag & ZHAVELOCK) + dl->dl_namelock = 1; + if ((flag & ZSHARED) && ++dl->dl_sharecnt > 1 && dl->dl_namesize == 0) { /* * We're the second shared reference to dl. Make a copy of @@ -325,7 +342,10 @@ zfs_dirent_unlock(zfs_dirlock_t *dl) zfs_dirlock_t **prev_dl, *cur_dl; mutex_enter(&dzp->z_lock); - rw_exit(&dzp->z_name_lock); + + if (!dl->dl_namelock) + rw_exit(&dzp->z_name_lock); + if (dl->dl_sharecnt > 1) { dl->dl_sharecnt--; mutex_exit(&dzp->z_lock); Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun May 16 07:16:28 2010 (r208130) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Sun May 16 07:46:03 2010 (r208131) @@ -3208,6 +3208,15 @@ top: } } + /* + * If the source and destination directories are the same, we should + * grab the z_name_lock of that directory only once. + */ + if (sdzp == tdzp) { + zflg |= ZHAVELOCK; + rw_enter(&sdzp->z_name_lock, RW_READER); + } + if (cmp < 0) { serr = zfs_dirent_lock(&sdl, sdzp, snm, &szp, ZEXISTS | zflg, NULL, NULL); @@ -3230,6 +3239,10 @@ top: if (tzp) VN_RELE(ZTOV(tzp)); } + + if (sdzp == tdzp) + rw_exit(&sdzp->z_name_lock); + if (strcmp(snm, ".") == 0 || strcmp(snm, "..") == 0) serr = EINVAL; ZFS_EXIT(zfsvfs); @@ -3238,6 +3251,10 @@ top: if (terr) { zfs_dirent_unlock(sdl); VN_RELE(ZTOV(szp)); + + if (sdzp == tdzp) + rw_exit(&sdzp->z_name_lock); + if (strcmp(tnm, "..") == 0) terr = EINVAL; ZFS_EXIT(zfsvfs); @@ -3320,6 +3337,10 @@ top: zfs_rename_unlock(&zl); zfs_dirent_unlock(sdl); zfs_dirent_unlock(tdl); + + if (sdzp == tdzp) + rw_exit(&sdzp->z_name_lock); + VN_RELE(ZTOV(szp)); if (tzp) VN_RELE(ZTOV(tzp)); @@ -3367,6 +3388,9 @@ out: zfs_dirent_unlock(sdl); zfs_dirent_unlock(tdl); + if (sdzp == tdzp) + rw_exit(&sdzp->z_name_lock); + VN_RELE(ZTOV(szp)); if (tzp) VN_RELE(ZTOV(tzp));