* [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init
@ 2022-10-19 8:37 Christoph Hellwig
2022-10-19 8:49 ` Guenter Roeck
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Christoph Hellwig @ 2022-10-19 8:37 UTC (permalink / raw)
To: kbusch, sagi, fancer.lancer, linux; +Cc: linux-nvme
An NVMe controller works perfectly fine even when the hwmon
initialization fails. Stop returning errors that do not come from a
controller reset from nvme_hwmon_init to handle this case consistently.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Changes since v1:
- switch to ignoring the errors in the caller.
drivers/nvme/host/core.c | 6 +++++-
drivers/nvme/host/hwmon.c | 13 ++++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9cbe7854d4883..dc42206005855 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3262,8 +3262,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
return ret;
if (!ctrl->identified && !nvme_discovery_ctrl(ctrl)) {
+ /*
+ * Do not return errors unless we are in a controller reset,
+ * the controller works perfectly fine without hwmon.
+ */
ret = nvme_hwmon_init(ctrl);
- if (ret < 0)
+ if (ret == -EINTR)
return ret;
}
diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
index 0a586d7129201..23918bb7bdca2 100644
--- a/drivers/nvme/host/hwmon.c
+++ b/drivers/nvme/host/hwmon.c
@@ -230,7 +230,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
- return 0;
+ return -ENOMEM;
data->ctrl = ctrl;
mutex_init(&data->read_lock);
@@ -238,8 +238,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
err = nvme_hwmon_get_smart_log(data);
if (err) {
dev_warn(dev, "Failed to read smart log (error %d)\n", err);
- kfree(data);
- return err;
+ goto err_free_data;
}
hwmon = hwmon_device_register_with_info(dev, "nvme",
@@ -247,11 +246,15 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
NULL);
if (IS_ERR(hwmon)) {
dev_warn(dev, "Failed to instantiate hwmon device\n");
- kfree(data);
- return PTR_ERR(hwmon);
+ err = PTR_ERR(hwmon);
+ goto err_free_data;
}
ctrl->hwmon_device = hwmon;
return 0;
+
+err_free_data:
+ kfree(data);
+ return err;
}
void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init
2022-10-19 8:37 [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init Christoph Hellwig
@ 2022-10-19 8:49 ` Guenter Roeck
2022-10-19 9:04 ` Serge Semin
2022-10-19 9:39 ` Sagi Grimberg
2 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2022-10-19 8:49 UTC (permalink / raw)
To: Christoph Hellwig, kbusch, sagi, fancer.lancer; +Cc: linux-nvme
On 10/19/22 01:37, Christoph Hellwig wrote:
> An NVMe controller works perfectly fine even when the hwmon
> initialization fails. Stop returning errors that do not come from a
> controller reset from nvme_hwmon_init to handle this case consistently.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
>
> Changes since v1:
> - switch to ignoring the errors in the caller.
>
> drivers/nvme/host/core.c | 6 +++++-
> drivers/nvme/host/hwmon.c | 13 ++++++++-----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9cbe7854d4883..dc42206005855 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3262,8 +3262,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
> return ret;
>
> if (!ctrl->identified && !nvme_discovery_ctrl(ctrl)) {
> + /*
> + * Do not return errors unless we are in a controller reset,
> + * the controller works perfectly fine without hwmon.
> + */
> ret = nvme_hwmon_init(ctrl);
> - if (ret < 0)
> + if (ret == -EINTR)
> return ret;
> }
>
> diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
> index 0a586d7129201..23918bb7bdca2 100644
> --- a/drivers/nvme/host/hwmon.c
> +++ b/drivers/nvme/host/hwmon.c
> @@ -230,7 +230,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
>
> data = kzalloc(sizeof(*data), GFP_KERNEL);
> if (!data)
> - return 0;
> + return -ENOMEM;
>
> data->ctrl = ctrl;
> mutex_init(&data->read_lock);
> @@ -238,8 +238,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> err = nvme_hwmon_get_smart_log(data);
> if (err) {
> dev_warn(dev, "Failed to read smart log (error %d)\n", err);
> - kfree(data);
> - return err;
> + goto err_free_data;
> }
>
> hwmon = hwmon_device_register_with_info(dev, "nvme",
> @@ -247,11 +246,15 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> NULL);
> if (IS_ERR(hwmon)) {
> dev_warn(dev, "Failed to instantiate hwmon device\n");
> - kfree(data);
> - return PTR_ERR(hwmon);
> + err = PTR_ERR(hwmon);
> + goto err_free_data;
> }
> ctrl->hwmon_device = hwmon;
> return 0;
> +
> +err_free_data:
> + kfree(data);
> + return err;
> }
>
> void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init
2022-10-19 8:37 [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init Christoph Hellwig
2022-10-19 8:49 ` Guenter Roeck
@ 2022-10-19 9:04 ` Serge Semin
2022-10-19 9:39 ` Sagi Grimberg
2 siblings, 0 replies; 5+ messages in thread
From: Serge Semin @ 2022-10-19 9:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kbusch, sagi, linux, linux-nvme
On Wed, Oct 19, 2022 at 10:37:44AM +0200, Christoph Hellwig wrote:
> An NVMe controller works perfectly fine even when the hwmon
> initialization fails. Stop returning errors that do not come from a
> controller reset from nvme_hwmon_init to handle this case consistently.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>
> Changes since v1:
> - switch to ignoring the errors in the caller.
Great! Thanks.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
-Sergey
>
> drivers/nvme/host/core.c | 6 +++++-
> drivers/nvme/host/hwmon.c | 13 ++++++++-----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9cbe7854d4883..dc42206005855 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3262,8 +3262,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
> return ret;
>
> if (!ctrl->identified && !nvme_discovery_ctrl(ctrl)) {
> + /*
> + * Do not return errors unless we are in a controller reset,
> + * the controller works perfectly fine without hwmon.
> + */
> ret = nvme_hwmon_init(ctrl);
> - if (ret < 0)
> + if (ret == -EINTR)
> return ret;
> }
>
> diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
> index 0a586d7129201..23918bb7bdca2 100644
> --- a/drivers/nvme/host/hwmon.c
> +++ b/drivers/nvme/host/hwmon.c
> @@ -230,7 +230,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
>
> data = kzalloc(sizeof(*data), GFP_KERNEL);
> if (!data)
> - return 0;
> + return -ENOMEM;
>
> data->ctrl = ctrl;
> mutex_init(&data->read_lock);
> @@ -238,8 +238,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> err = nvme_hwmon_get_smart_log(data);
> if (err) {
> dev_warn(dev, "Failed to read smart log (error %d)\n", err);
> - kfree(data);
> - return err;
> + goto err_free_data;
> }
>
> hwmon = hwmon_device_register_with_info(dev, "nvme",
> @@ -247,11 +246,15 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> NULL);
> if (IS_ERR(hwmon)) {
> dev_warn(dev, "Failed to instantiate hwmon device\n");
> - kfree(data);
> - return PTR_ERR(hwmon);
> + err = PTR_ERR(hwmon);
> + goto err_free_data;
> }
> ctrl->hwmon_device = hwmon;
> return 0;
> +
> +err_free_data:
> + kfree(data);
> + return err;
> }
>
> void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init
2022-10-19 8:37 [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init Christoph Hellwig
2022-10-19 8:49 ` Guenter Roeck
2022-10-19 9:04 ` Serge Semin
@ 2022-10-19 9:39 ` Sagi Grimberg
2022-10-19 10:41 ` Christoph Hellwig
2 siblings, 1 reply; 5+ messages in thread
From: Sagi Grimberg @ 2022-10-19 9:39 UTC (permalink / raw)
To: Christoph Hellwig, kbusch, fancer.lancer, linux; +Cc: linux-nvme
> An NVMe controller works perfectly fine even when the hwmon
> initialization fails. Stop returning errors that do not come from a
> controller reset from nvme_hwmon_init to handle this case consistently.
Maybe I'm missing something, but nvme_hwmon_init is not called
from the reset flow, only in the first init, I don't understand
how the commit msg follows the code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>
> Changes since v1:
> - switch to ignoring the errors in the caller.
>
> drivers/nvme/host/core.c | 6 +++++-
> drivers/nvme/host/hwmon.c | 13 ++++++++-----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9cbe7854d4883..dc42206005855 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3262,8 +3262,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
> return ret;
>
> if (!ctrl->identified && !nvme_discovery_ctrl(ctrl)) {
> + /*
> + * Do not return errors unless we are in a controller reset,
> + * the controller works perfectly fine without hwmon.
> + */
> ret = nvme_hwmon_init(ctrl);
> - if (ret < 0)
> + if (ret == -EINTR)
> return ret;
> }
>
> diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
> index 0a586d7129201..23918bb7bdca2 100644
> --- a/drivers/nvme/host/hwmon.c
> +++ b/drivers/nvme/host/hwmon.c
> @@ -230,7 +230,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
>
> data = kzalloc(sizeof(*data), GFP_KERNEL);
> if (!data)
> - return 0;
> + return -ENOMEM;
>
> data->ctrl = ctrl;
> mutex_init(&data->read_lock);
> @@ -238,8 +238,7 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> err = nvme_hwmon_get_smart_log(data);
> if (err) {
> dev_warn(dev, "Failed to read smart log (error %d)\n", err);
> - kfree(data);
> - return err;
> + goto err_free_data;
> }
>
> hwmon = hwmon_device_register_with_info(dev, "nvme",
> @@ -247,11 +246,15 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> NULL);
> if (IS_ERR(hwmon)) {
> dev_warn(dev, "Failed to instantiate hwmon device\n");
> - kfree(data);
> - return PTR_ERR(hwmon);
> + err = PTR_ERR(hwmon);
> + goto err_free_data;
> }
> ctrl->hwmon_device = hwmon;
> return 0;
> +
> +err_free_data:
> + kfree(data);
> + return err;
> }
>
> void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init
2022-10-19 9:39 ` Sagi Grimberg
@ 2022-10-19 10:41 ` Christoph Hellwig
0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2022-10-19 10:41 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Christoph Hellwig, kbusch, fancer.lancer, linux, linux-nvme
On Wed, Oct 19, 2022 at 12:39:08PM +0300, Sagi Grimberg wrote:
>
>> An NVMe controller works perfectly fine even when the hwmon
>> initialization fails. Stop returning errors that do not come from a
>> controller reset from nvme_hwmon_init to handle this case consistently.
>
> Maybe I'm missing something, but nvme_hwmon_init is not called
> from the reset flow, only in the first init, I don't understand
> how the commit msg follows the code.
If the controller is reset during initialization we'll get EINTR
here.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-19 10:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-19 8:37 [PATCH v2] nvme-hwmon: consistently ignore errors from nvme_hwmon_init Christoph Hellwig
2022-10-19 8:49 ` Guenter Roeck
2022-10-19 9:04 ` Serge Semin
2022-10-19 9:39 ` Sagi Grimberg
2022-10-19 10:41 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox