diff mbox series

[v10,17/18] scsi: ufs: Forbid auto-hibernation without I/O scheduler

Message ID 20230818193546.2014874-18-bvanassche@acm.org (mailing list archive)
State New, archived
Headers show
Series Improve performance for zoned UFS devices | expand

Commit Message

Bart Van Assche Aug. 18, 2023, 7:34 p.m. UTC
UFSHCI 3.0 controllers do not preserve the write order if auto-hibernation
is enabled. If the write order is not preserved, an I/O scheduler is
required to serialize zoned writes. Hence do not allow auto-hibernation
to be enabled without I/O scheduler if a zoned logical unit is present
and if the controller is operating in legacy mode. This patch has been
tested with the following shell script:

    show_ah8() {
        echo -n "auto_hibern8: "
        adb shell "cat /sys/devices/platform/13200000.ufs/auto_hibern8"
    }

    set_ah8() {
        local rc
        adb shell "echo $1 > /sys/devices/platform/13200000.ufs/auto_hibern8"
        rc=$?
        show_ah8
        return $rc
    }

    set_iosched() {
        adb shell "echo $1 >/sys/class/block/$zoned_bdev/queue/scheduler &&
    	           echo -n 'I/O scheduler: ' &&
	           cat /sys/class/block/sde/queue/scheduler"
    }

    adb root
    zoned_bdev=$(adb shell grep -lvw 0 /sys/class/block/sd*/queue/chunk_sectors |&
	         sed 's|/sys/class/block/||g;s|/queue/chunk_sectors||g')
    [ -n "$zoned_bdev" ]
    show_ah8
    set_ah8 0
    set_iosched none
    if set_ah8 150000; then
        echo "Error: enabled AH8 without I/O scheduler"
    fi
    set_iosched mq-deadline
    set_ah8 150000

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Can Guo <quic_cang@quicinc.com>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 58 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Bao D. Nguyen Aug. 18, 2023, 9:17 p.m. UTC | #1
On 8/18/2023 12:34 PM, Bart Van Assche wrote:
> UFSHCI 3.0 controllers do not preserve the write order if auto-hibernation
> is enabled. If the write order is not preserved, an I/O scheduler is
> required to serialize zoned writes. Hence do not allow auto-hibernation
> to be enabled without I/O scheduler if a zoned logical unit is present
> and if the controller is operating in legacy mode. This patch has been
> tested with the following shell script:
> 

Reviewed-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Can Guo Aug. 21, 2023, 5:44 a.m. UTC | #2
On 8/19/2023 3:34 AM, Bart Van Assche wrote:
> UFSHCI 3.0 controllers do not preserve the write order if auto-hibernation
> is enabled. If the write order is not preserved, an I/O scheduler is
> required to serialize zoned writes. Hence do not allow auto-hibernation
> to be enabled without I/O scheduler if a zoned logical unit is present
> and if the controller is operating in legacy mode. This patch has been
> tested with the following shell script:
>
>      show_ah8() {
>          echo -n "auto_hibern8: "
>          adb shell "cat /sys/devices/platform/13200000.ufs/auto_hibern8"
>      }
>
>      set_ah8() {
>          local rc
>          adb shell "echo $1 > /sys/devices/platform/13200000.ufs/auto_hibern8"
>          rc=$?
>          show_ah8
>          return $rc
>      }
>
>      set_iosched() {
>          adb shell "echo $1 >/sys/class/block/$zoned_bdev/queue/scheduler &&
>      	           echo -n 'I/O scheduler: ' &&
> 	           cat /sys/class/block/sde/queue/scheduler"
>      }
>
>      adb root
>      zoned_bdev=$(adb shell grep -lvw 0 /sys/class/block/sd*/queue/chunk_sectors |&
> 	         sed 's|/sys/class/block/||g;s|/queue/chunk_sectors||g')
>      [ -n "$zoned_bdev" ]
>      show_ah8
>      set_ah8 0
>      set_iosched none
>      if set_ah8 150000; then
>          echo "Error: enabled AH8 without I/O scheduler"
>      fi
>      set_iosched mq-deadline
>      set_ah8 150000
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Can Guo <quic_cang@quicinc.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/ufs/core/ufshcd.c | 58 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 58 insertions(+)

Reviewed-by: Can Guo <quic_cang@quicinc.com>
Can Guo Aug. 21, 2023, 7:13 a.m. UTC | #3
On 8/19/2023 3:34 AM, Bart Van Assche wrote:
> UFSHCI 3.0 controllers do not preserve the write order if auto-hibernation
> is enabled. If the write order is not preserved, an I/O scheduler is
> required to serialize zoned writes. Hence do not allow auto-hibernation
> to be enabled without I/O scheduler if a zoned logical unit is present
> and if the controller is operating in legacy mode. This patch has been
> tested with the following shell script:
>
>      show_ah8() {
>          echo -n "auto_hibern8: "
>          adb shell "cat /sys/devices/platform/13200000.ufs/auto_hibern8"
>      }
>
>      set_ah8() {
>          local rc
>          adb shell "echo $1 > /sys/devices/platform/13200000.ufs/auto_hibern8"
>          rc=$?
>          show_ah8
>          return $rc
>      }
>
>      set_iosched() {
>          adb shell "echo $1 >/sys/class/block/$zoned_bdev/queue/scheduler &&
>      	           echo -n 'I/O scheduler: ' &&
> 	           cat /sys/class/block/sde/queue/scheduler"
>      }
>
>      adb root
>      zoned_bdev=$(adb shell grep -lvw 0 /sys/class/block/sd*/queue/chunk_sectors |&
> 	         sed 's|/sys/class/block/||g;s|/queue/chunk_sectors||g')
>      [ -n "$zoned_bdev" ]
>      show_ah8
>      set_ah8 0
>      set_iosched none
>      if set_ah8 150000; then
>          echo "Error: enabled AH8 without I/O scheduler"
>      fi
>      set_iosched mq-deadline
>      set_ah8 150000
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Can Guo <quic_cang@quicinc.com>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/ufs/core/ufshcd.c | 58 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 58 insertions(+)
>
Reviewed-by: Can Guo <quic_cang@quicinc.com>
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 68bf8e94eee6..c28a362b5b99 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4337,6 +4337,30 @@  int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
 }
 EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_exit);
 
+static int ufshcd_update_preserves_write_order(struct ufs_hba *hba,
+					       bool preserves_write_order)
+{
+	struct scsi_device *sdev;
+
+	if (!preserves_write_order) {
+		shost_for_each_device(sdev, hba->host) {
+			struct request_queue *q = sdev->request_queue;
+
+			/*
+			 * Refuse to enable auto-hibernation if no I/O scheduler
+			 * is present. This code does not check whether the
+			 * attached I/O scheduler serializes zoned writes
+			 * (ELEVATOR_F_ZBD_SEQ_WRITE) because this cannot be
+			 * checked from outside the block layer core.
+			 */
+			if (blk_queue_is_zoned(q) && !q->elevator)
+				return -EPERM;
+		}
+	}
+
+	return 0;
+}
+
 static void ufshcd_configure_auto_hibern8(struct ufs_hba *hba)
 {
 	if (!ufshcd_is_auto_hibern8_supported(hba))
@@ -4345,13 +4369,40 @@  static void ufshcd_configure_auto_hibern8(struct ufs_hba *hba)
 	ufshcd_writel(hba, hba->ahit, REG_AUTO_HIBERNATE_IDLE_TIMER);
 }
 
+/**
+ * ufshcd_auto_hibern8_update() - Modify the auto-hibernation control register
+ * @hba: per-adapter instance
+ * @ahit: New auto-hibernate settings. Includes the scale and the value of the
+ * auto-hibernation timer. See also the UFSHCI_AHIBERN8_TIMER_MASK and
+ * UFSHCI_AHIBERN8_SCALE_MASK constants.
+ *
+ * A note: UFSHCI controllers do not preserve the command order in legacy mode
+ * if auto-hibernation is enabled. If the command order is not preserved, an
+ * I/O scheduler that serializes zoned writes (mq-deadline) is required if a
+ * zoned logical unit is present. Enabling auto-hibernation without attaching
+ * the mq-deadline scheduler first may cause unaligned write errors for the
+ * zoned logical unit if a zoned logical unit is present.
+ */
 int ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
 {
 	const u32 cur_ahit = READ_ONCE(hba->ahit);
+	bool prev_state, new_state;
+	int ret;
 
 	if (!ufshcd_is_auto_hibern8_supported(hba) || cur_ahit == ahit)
 		return 0;
 
+	prev_state = FIELD_GET(UFSHCI_AHIBERN8_TIMER_MASK, cur_ahit);
+	new_state = FIELD_GET(UFSHCI_AHIBERN8_TIMER_MASK, ahit);
+
+	if (!is_mcq_enabled(hba) && !prev_state && new_state) {
+		/*
+		 * Auto-hibernation will be enabled for legacy UFSHCI mode.
+		 */
+		ret = ufshcd_update_preserves_write_order(hba, false);
+		if (ret)
+			return ret;
+	}
 	WRITE_ONCE(hba->ahit, ahit);
 	if (!pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
 		ufshcd_rpm_get_sync(hba);
@@ -4360,6 +4411,13 @@  int ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
 		ufshcd_release(hba);
 		ufshcd_rpm_put_sync(hba);
 	}
+	if (!is_mcq_enabled(hba) && prev_state && !new_state) {
+		/*
+		 * Auto-hibernation has been disabled.
+		 */
+		ret = ufshcd_update_preserves_write_order(hba, true);
+		WARN_ON_ONCE(ret);
+	}
 
 	return 0;
 }