vdev_disk_open: fix ENOENT retry & cleanup error handling

In 88261a83ae I reorganized the blockdev open error handling to
accommodate the permission check. What I didn't notice was that I'd made
the ENOENT case break after the timeout, rather than looping and
retrying.

This commit fixes it up the right way, and this time adds a lot more
commentary to make it easier to see what's happening for each case.

Sponsored-by: TrueNAS
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ameer Hamza <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes #18993

module/os/linux/zfs/vdev_disk.c

@@ -414,7 +414,7 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize,
 	 */
 
 	hrtime_t start = gethrtime();
-	int err = ENXIO;
+	int err = -ENXIO;
 	while (err != 0 && ((gethrtime() - start) < timeout)) {
 
 		/*
@@ -425,7 +425,7 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize,
 		 */
 		struct path devpath;
 		err = kern_path(v->vdev_path, LOOKUP_FOLLOW, &devpath);
-		if (err == 0) {
+		if (likely(err == 0)) {
 			int mask =
 			    (smode & SPA_MODE_READ ? MAY_READ : 0) |
 			    (smode & SPA_MODE_WRITE ? MAY_WRITE : 0);
@@ -433,29 +433,46 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize,
 			path_put(&devpath);
 		}
 
-		if (err == 0) {
+		if (likely(err == 0)) {
+			/*
+			 * Device node exists and we have access to it, so try
+			 * to open it.
+			 */
 			bdh = vdev_blkdev_get_by_path(v->vdev_path, smode,
 			    zfs_vdev_holder);
-			err = BDH_IS_ERR(bdh) ? BDH_PTR_ERR(bdh) : 0;
-		}
 
-		if (err == 0)
-			break;
-
-		if (unlikely(err == -ENOENT)) {
-			/*
-			 * There is no point of waiting since device is removed
-			 * explicitly
-			 */
-			if (v->vdev_removed)
+			if (likely(!BDH_IS_ERR(bdh)))
+				/* Device open, nothing more to consider. */
 				break;
 
+			err = BDH_PTR_ERR(bdh);
+		}
+
+		ASSERT3U(err, !=, 0);
+
+		if (err == -ENOENT) {
+			/* Device node disappeared, see above comment. */
+
+			if (v->vdev_removed) {
+				/*
+				 * There is no point of waiting since device is
+				 * removed explicitly
+				 */
+				break;
+			}
+
+			/* Wait a moment, then retry. */
 			schedule_timeout_interruptible(MSEC_TO_TICK(10));
-		} else if (unlikely(err == -ERESTARTSYS)) {
+			continue;
+		}
+
+		if (err == -ERESTARTSYS) {
+			/* zvol deadlock avoided, extend timeout and retry. */
 			timeout = MSEC2NSEC(zfs_vdev_open_timeout_ms * 10);
 			continue;
 		}
 
+		/* Consider all other errors permanent, no retry. */
 		break;
 	}