On 2/20/21 12:16 AM, Keith Busch wrote:

On Fri, Feb 19, 2021 at 03:46:25PM +0100, Hannes Reinecke wrote:

quoted

In some setups like RAID we need to return an I/O error
once all paths are unavailable to allow the upper layers
to start their own error recovery (like redirecting I/O
to other mirrors).
This patch adds a sysfs attribute 'fail_if_no_path' to
allow the admin to enable that behaviour instead of the
current 'queue until a path becomes available' policy.

Signed-off-by: Hannes Reinecke <hare@suse.de>

Sounds okay to me, just some minor nits below.

quoted

@@ -283,10 +283,13 @@ static bool nvme_available_path(struct nvme_ns_head *head)
  			continue;
  		switch (ns->ctrl->state) {
  		case NVME_CTRL_LIVE:
+			return true;
  		case NVME_CTRL_RESETTING:
  		case NVME_CTRL_CONNECTING:
  			/* fallthru */
-			return true;
+			if (!test_bit(NVME_NSHEAD_FAIL_IF_NO_PATH,
+				      &head->flags))
+				return true;

It doesn't look like the 'fallthru' is in the right place anymore,
though I can't tell why it is there in the first place.

Indeed, the 'fallthru' is misplaced; it needs to go after
NVME_CTRL_RESETTING.

quoted

  		default:
  			break;
  		}

@@ -641,6 +644,37 @@ static ssize_t ana_state_show(struct device *dev, struct device_attribute *attr,
  }
  DEVICE_ATTR_RO(ana_state);
  
+static ssize_t fail_if_no_path_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	struct nvme_ns_head *head = disk->private_data;
+
+	return sprintf(buf, "%d\n",
+		       test_bit(NVME_NSHEAD_FAIL_IF_NO_PATH, &head->flags) ?
+		       1 : 0);
+}
+
+static ssize_t fail_if_no_path_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	struct nvme_ns_head *head = disk->private_data;
+	int fail_if_no_path, err;
+
+	err = kstrtoint(buf, 10, &fail_if_no_path);
+	if (err)
+		return -EINVAL;
+
+	else if (fail_if_no_path <= 0)
+		clear_bit(NVME_NSHEAD_FAIL_IF_NO_PATH, &head->flags);
+	else
+		set_bit(NVME_NSHEAD_FAIL_IF_NO_PATH, &head->flags);

The extra line above the 'else if' should be removed, or just change it
to simply 'if'.

Sure.

I'll be resending.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme