d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix deadlock between zfs_dirent_lock and zfs_rmdir

Browse Source 
OpenSolaris onnv revision:	11321:506b7043a14c

Approved by:	pjd, delphij (mentor)
Obtained from:	OpenSolaris (Bug ID 6847615)
MFC after:	3 days
This commit is contained in:
Martin Matuska 2010-05-16 07:46:03 +00:00
parent db708a6e2c
commit ee56d88b76
4 changed files with 50 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_dir.h
View File

@ -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 */

1
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_znode.h
View File

@ -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 */

28
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_dir.c
View File

@ -114,6 +114,8 @@ zfs_match_find(zfsvfs_t *zfsvfs, znode_t *dzp, char *name, boolean_t exact,
* 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, znode_t *dzp, char *name, znode_t **zpp,
/*
* 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, znode_t *dzp, char *name, znode_t **zpp,
}
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, znode_t *dzp, char *name, znode_t **zpp,
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, znode_t *dzp, char *name, znode_t **zpp,
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);

24
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
View File

@ -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));