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));
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to