zv_suspend_lock in zvol_open()/zvol_release()

Acquire zv_suspend_lock on first open and last close only.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Boris Protopopov <[email protected]>
Closes #6342 

1 files changed, 41 insertions, 23 deletions

diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c
index 254760220..40fdf0777 100644
--- a/module/zfs/zvol.c
+++ b/module/zfs/zvol.c
@@ -1304,9 +1304,9 @@ zvol_open(struct block_device *bdev, fmode_t flag)
 {
 	zvol_state_t *zv;
 	int error = 0;
-	boolean_t drop_suspend = B_FALSE;
+	boolean_t drop_suspend = B_TRUE;
 
-	ASSERT(!mutex_owned(&zvol_state_lock));
+	ASSERT(!MUTEX_HELD(&zvol_state_lock));
 
 	mutex_enter(&zvol_state_lock);
 	/*
@@ -1321,23 +1321,31 @@ zvol_open(struct block_device *bdev, fmode_t flag)
 		return (SET_ERROR(-ENXIO));
 	}
 
-	/* take zv_suspend_lock before zv_state_lock */
-	rw_enter(&zv->zv_suspend_lock, RW_READER);
-
 	mutex_enter(&zv->zv_state_lock);
-
 	/*
 	 * make sure zvol is not suspended during first open
-	 * (hold zv_suspend_lock), otherwise, drop the lock
+	 * (hold zv_suspend_lock) and respect proper lock acquisition
+	 * ordering - zv_suspend_lock before zv_state_lock
 	 */
 	if (zv->zv_open_count == 0) {
-		drop_suspend = B_TRUE;
+		if (!rw_tryenter(&zv->zv_suspend_lock, RW_READER)) {
+			mutex_exit(&zv->zv_state_lock);
+			rw_enter(&zv->zv_suspend_lock, RW_READER);
+			mutex_enter(&zv->zv_state_lock);
+			/* check to see if zv_suspend_lock is needed */
+			if (zv->zv_open_count != 0) {
+				rw_exit(&zv->zv_suspend_lock);
+				drop_suspend = B_FALSE;
+			}
+		}
 	} else {
-		rw_exit(&zv->zv_suspend_lock);
+		drop_suspend = B_FALSE;
 	}
-
 	mutex_exit(&zvol_state_lock);
 
+	ASSERT(MUTEX_HELD(&zv->zv_state_lock));
+	ASSERT(zv->zv_open_count != 0 || RW_READ_HELD(&zv->zv_suspend_lock));
+
 	if (zv->zv_open_count == 0) {
 		error = zvol_first_open(zv);
 		if (error)
@@ -1374,28 +1382,38 @@ static int
 zvol_release(struct gendisk *disk, fmode_t mode)
 {
 	zvol_state_t *zv;
-	boolean_t drop_suspend = B_FALSE;
+	boolean_t drop_suspend = B_TRUE;
 
-	ASSERT(!mutex_owned(&zvol_state_lock));
+	ASSERT(!MUTEX_HELD(&zvol_state_lock));
 
 	mutex_enter(&zvol_state_lock);
 	zv = disk->private_data;
-	ASSERT(zv && zv->zv_open_count > 0);
-
-	/* take zv_suspend_lock before zv_state_lock */
-	rw_enter(&zv->zv_suspend_lock, RW_READER);
 
 	mutex_enter(&zv->zv_state_lock);
-	mutex_exit(&zvol_state_lock);
-
+	ASSERT(zv->zv_open_count > 0);
 	/*
 	 * make sure zvol is not suspended during last close
-	 * (hold zv_suspend_lock), otherwise, drop the lock
+	 * (hold zv_suspend_lock) and respect proper lock acquisition
+	 * ordering - zv_suspend_lock before zv_state_lock
 	 */
-	if (zv->zv_open_count == 1)
-		drop_suspend = B_TRUE;
-	else
-		rw_exit(&zv->zv_suspend_lock);
+	if (zv->zv_open_count == 1) {
+		if (!rw_tryenter(&zv->zv_suspend_lock, RW_READER)) {
+			mutex_exit(&zv->zv_state_lock);
+			rw_enter(&zv->zv_suspend_lock, RW_READER);
+			mutex_enter(&zv->zv_state_lock);
+			/* check to see if zv_suspend_lock is needed */
+			if (zv->zv_open_count != 1) {
+				rw_exit(&zv->zv_suspend_lock);
+				drop_suspend = B_FALSE;
+			}
+		}
+	} else {
+		drop_suspend = B_FALSE;
+	}
+	mutex_exit(&zvol_state_lock);
+
+	ASSERT(MUTEX_HELD(&zv->zv_state_lock));
+	ASSERT(zv->zv_open_count != 1 || RW_READ_HELD(&zv->zv_suspend_lock));
 
 	zv->zv_open_count--;
 	if (zv->zv_open_count == 0)