Changes in v2

Add mutex_lock in nvme_set_latency_tolerance() for Potential competition

between nvme_set_latency_tolerance() and apst_update_store().

Changes in v3

In PACH nvme: add sysfs interface for APST table updates,Add why dynamic APST

updates are needed in the commit message, fix code formatting issues.

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

Currently the function fails silently on PST table memory allocation failure.

Add warning messages to improve error visibility.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>

---

drivers/nvme/host/core.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/core.c

+++ b/drivers/nvme/host/core.c

@@ -XXX,XX +XXX,XX @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl)

    }

    table = kzalloc(sizeof(*table), GFP_KERNEL);

-    if (!table)

+    if (!table) {

+        dev_warn(ctrl->device, "Failed to allocate pst table; not using APST\n");

        return 0;

+    }

    if (!ctrl->apst_enabled || ctrl->ps_max_latency_us == 0) {

        /* Turn off APST. */

--

2.25.1

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

In desktop systems, users can choose between power-saving mode and

performance mode. These two modes involve different trade-offs between

NVMe performance and power efficiency, thus requiring dynamic updates to APST.

Currently, the APST (Autonomous Power State Transition) table can only be

updated during module initialization via module parameters or indirectly

by setting QoS latency requirements. This patch adds a direct sysfs

interface to allow dynamic updates to the APST table at runtime.

The new sysfs entry is created at:

/sys/class/nvme/<controller>/apst_update

This provides more flexibility in power management tuning without

requiring module reload or QoS latency changes.

Example usage:

update nvme module parameters.

echo 1 > /sys/class/nvme/nvme0/apst_update

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>

---

drivers/nvme/host/core.c | 9 +++++++--

drivers/nvme/host/nvme.h | 2 ++

drivers/nvme/host/sysfs.c | 24 ++++++++++++++++++++++++

3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/core.c

+++ b/drivers/nvme/host/core.c

@@ -XXX,XX +XXX,XX @@ static bool nvme_apst_get_transition_time(u64 total_latency,

*

* Users can set ps_max_latency_us to zero to turn off APST.

*/

-static int nvme_configure_apst(struct nvme_ctrl *ctrl)

+int nvme_configure_apst(struct nvme_ctrl *ctrl)

{

    struct nvme_feat_auto_pst *table;

    unsigned apste = 0;

@@ -XXX,XX +XXX,XX @@ static void nvme_set_latency_tolerance(struct device *dev, s32 val)

    if (ctrl->ps_max_latency_us != latency) {

        ctrl->ps_max_latency_us = latency;

-        if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE)

+        if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {

+            mutex_lock(&ctrl->apst_lock);

            nvme_configure_apst(ctrl);

+            mutex_unlock(&ctrl->apst_lock);

+        }

    }

}

@@ -XXX,XX +XXX,XX @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,

    ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;

    ctrl->ka_last_check_time = jiffies;

+    mutex_init(&ctrl->apst_lock);

+

    BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >

            PAGE_SIZE);

    ctrl->discard_page = alloc_page(GFP_KERNEL);

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/nvme.h

+++ b/drivers/nvme/host/nvme.h

@@ -XXX,XX +XXX,XX @@ struct nvme_ctrl {

    key_serial_t tls_pskid;

    /* Power saving configuration */

+    struct mutex apst_lock;

    u64 ps_max_latency_us;

    bool apst_enabled;

@@ -XXX,XX +XXX,XX @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);

void nvme_wait_freeze(struct nvme_ctrl *ctrl);

int nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);

void nvme_start_freeze(struct nvme_ctrl *ctrl);

+int nvme_configure_apst(struct nvme_ctrl *ctrl);

static inline enum req_op nvme_req_op(struct nvme_command *cmd)

{

diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/sysfs.c

+++ b/drivers/nvme/host/sysfs.c

@@ -XXX,XX +XXX,XX @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,

    nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);

#endif

+static ssize_t apst_update_store(struct device *dev,

+                     struct device_attribute *attr,

+                     const char *buf, size_t size)

+{

+    struct nvme_ctrl *ctrl = dev_get_drvdata(dev);

+    bool bool_data = false;

+    int err;

+

+    err = kstrtobool(buf, &bool_data);

+

+    if (err)

+        return err;

+

+    if (bool_data && nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE) {

+        mutex_lock(&ctrl->apst_lock);

+        nvme_configure_apst(ctrl);

+        mutex_unlock(&ctrl->apst_lock);

+    }

+

+    return size;

+}

+static DEVICE_ATTR_WO(apst_update);

+

static struct attribute *nvme_dev_attrs[] = {

    &dev_attr_reset_controller.attr,

    &dev_attr_rescan_controller.attr,

@@ -XXX,XX +XXX,XX @@ static struct attribute *nvme_dev_attrs[] = {

    &dev_attr_dhchap_ctrl_secret.attr,

#endif

    &dev_attr_adm_passthru_err_log_enabled.attr,

+    &dev_attr_apst_update.attr,

    NULL

};

--

2.25.1

From: Yaxiong Tian <tianyaxiong@kylinos.cn>

Currently, APST (Autonomous Power State Transition) parameters are

configured as module parameters affecting all controllers uniformly,

This lacks flexibility for heterogeneous systems.

This patch introduces per-controller sysfs attributes under each NVMe

controller's sysfs directory:

- apst_primary_timeout_ms

- apst_secondary_timeout_ms

- apst_primary_latency_tol_us

- apst_secondary_latency_tol_us

The attributes allow runtime configuration of: Timeout values for

primary/secondary states.

The existing module parameters are retained for backward compatibility

but now only serve as default values for new controllers.

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>

---

drivers/nvme/host/core.c | 16 ++++++++++------

drivers/nvme/host/nvme.h | 4 ++++

drivers/nvme/host/sysfs.c | 38 ++++++++++++++++++++++++++++++++++++++

3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/core.c

+++ b/drivers/nvme/host/core.c

@@ -XXX,XX +XXX,XX @@ static int nvme_configure_host_options(struct nvme_ctrl *ctrl)

* timeout value is returned and the matching tolerance index (1 or 2) is

* reported.

*/

-static bool nvme_apst_get_transition_time(u64 total_latency,

+static bool nvme_apst_get_transition_time(struct nvme_ctrl *ctrl, u64 total_latency,

        u64 *transition_time, unsigned *last_index)

{

-    if (total_latency <= apst_primary_latency_tol_us) {

+    if (total_latency <= ctrl->apst_primary_latency_tol_us) {

        if (*last_index == 1)

            return false;

        *last_index = 1;

-        *transition_time = apst_primary_timeout_ms;

+        *transition_time = ctrl->apst_primary_timeout_ms;

        return true;

    }

    if (apst_secondary_timeout_ms &&

-        total_latency <= apst_secondary_latency_tol_us) {

+        total_latency <= ctrl->apst_secondary_latency_tol_us) {

        if (*last_index <= 2)

            return false;

        *last_index = 2;

-        *transition_time = apst_secondary_timeout_ms;

+        *transition_time = ctrl->apst_secondary_timeout_ms;

        return true;

    }

    return false;

@@ -XXX,XX +XXX,XX @@ int nvme_configure_apst(struct nvme_ctrl *ctrl)

         * for higher power states.

         */

        if (apst_primary_timeout_ms && apst_primary_latency_tol_us) {

-            if (!nvme_apst_get_transition_time(total_latency_us,

+            if (!nvme_apst_get_transition_time(ctrl, total_latency_us,

                    &transition_ms, &last_lt_index))

                continue;

        } else {

@@ -XXX,XX +XXX,XX @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,

    ctrl->ka_last_check_time = jiffies;

    mutex_init(&ctrl->apst_lock);

+    ctrl->apst_primary_timeout_ms = apst_primary_timeout_ms;

+    ctrl->apst_secondary_timeout_ms = apst_secondary_timeout_ms;

+    ctrl->apst_primary_latency_tol_us = apst_primary_latency_tol_us;

+    ctrl->apst_secondary_latency_tol_us = apst_secondary_latency_tol_us;

    BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >

            PAGE_SIZE);

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/nvme.h

+++ b/drivers/nvme/host/nvme.h

@@ -XXX,XX +XXX,XX @@ struct nvme_ctrl {

    /* Power saving configuration */

    struct mutex apst_lock;

    u64 ps_max_latency_us;

+    u64 apst_primary_timeout_ms;

+    u64 apst_secondary_timeout_ms;

+    u64 apst_primary_latency_tol_us;

+    u64 apst_secondary_latency_tol_us;

    bool apst_enabled;

    /* PCIe only: */

diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c

index XXXXXXX..XXXXXXX 100644

--- a/drivers/nvme/host/sysfs.c

+++ b/drivers/nvme/host/sysfs.c

@@ -XXX,XX +XXX,XX @@ static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,

    nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);

#endif

+#define nvme_apst_show_and_store_function(field)                \

+static ssize_t field##_show(struct device *dev,        \

+             struct device_attribute *attr, char *buf)    \

+{                                    \

+    struct nvme_ctrl *ctrl = dev_get_drvdata(dev);            \

+\

+    return sysfs_emit(buf, "%llu\n", ctrl->field);    \

+}                                    \

+                                    \

+static ssize_t field##_store(struct device *dev,        \

+             struct device_attribute *attr,        \

+             const char *buf, size_t size)        \

+{                                    \

+    struct nvme_ctrl *ctrl = dev_get_drvdata(dev);            \

+    u64 data = 0;                    \

+    int err;                        \

+\

+    err = kstrtou64(buf, 0, &data);        \

+    if (err < 0)                        \

+        return err;                        \

+                            \

+    mutex_lock(&ctrl->apst_lock);        \

+    ctrl->field = data;     \

+    mutex_unlock(&ctrl->apst_lock);        \

+    return size;            \

+}                                    \

+static DEVICE_ATTR_RW(field);

+

+nvme_apst_show_and_store_function(apst_primary_timeout_ms);

+nvme_apst_show_and_store_function(apst_secondary_timeout_ms);

+nvme_apst_show_and_store_function(apst_primary_latency_tol_us);

+nvme_apst_show_and_store_function(apst_secondary_latency_tol_us);

+

static ssize_t apst_update_store(struct device *dev,

                     struct device_attribute *attr,

                     const char *buf, size_t size)

@@ -XXX,XX +XXX,XX @@ static struct attribute *nvme_dev_attrs[] = {

    &dev_attr_dhchap_ctrl_secret.attr,

#endif

    &dev_attr_adm_passthru_err_log_enabled.attr,

+

+    &dev_attr_apst_primary_timeout_ms.attr,

+    &dev_attr_apst_secondary_timeout_ms.attr,

+    &dev_attr_apst_primary_latency_tol_us.attr,

+    &dev_attr_apst_secondary_latency_tol_us.attr,

    &dev_attr_apst_update.attr,

    NULL

};

--

2.25.1