* [PATCH] nvme/hwmon: rework to avoid devm allocation
@ 2021-01-19 6:22 Hannes Reinecke
2021-01-19 6:28 ` Christoph Hellwig
0 siblings, 1 reply; 2+ messages in thread
From: Hannes Reinecke @ 2021-01-19 6:22 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-nvme, Daniel Wagner, Sagi Grimberg, Keith Busch, Hannes Reinecke
The original design to use device-managed resource allocation
doesn't really work as the NVMe controller has a vastly different
lifetime than the hwmon sysfs attributes, causing warning about
duplicate sysfs entries upon reconnection.
This patch reworks the hwmon allocation to avoid device-managed
resource allocation, and uses the NVMe controller as parent for
the sysfs attributes.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Tested-by: Daniel Wagner <dwagner@suse.de>
---
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/hwmon.c | 31 +++++++++++++++++++++----------
drivers/nvme/host/nvme.h | 6 ++++++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index fff49e544fdf..3c6c77e44bf7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4419,6 +4419,7 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
{
+ nvme_hwmon_exit(ctrl);
nvme_fault_inject_fini(&ctrl->fault_inject);
dev_pm_qos_hide_latency_tolerance(ctrl->device);
cdev_device_del(&ctrl->cdev, ctrl->device);
diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
index 552dbc04567b..8f9e96986780 100644
--- a/drivers/nvme/host/hwmon.c
+++ b/drivers/nvme/host/hwmon.c
@@ -223,12 +223,12 @@ static const struct hwmon_chip_info nvme_hwmon_chip_info = {
int nvme_hwmon_init(struct nvme_ctrl *ctrl)
{
- struct device *dev = ctrl->dev;
+ struct device *dev = ctrl->device;
struct nvme_hwmon_data *data;
struct device *hwmon;
int err;
- data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return 0;
@@ -237,19 +237,30 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
err = nvme_hwmon_get_smart_log(data);
if (err) {
- dev_warn(ctrl->device,
- "Failed to read smart log (error %d)\n", err);
- devm_kfree(dev, data);
+ dev_warn(dev, "Failed to read smart log (error %d)\n", err);
+ kfree(data);
return err;
}
- hwmon = devm_hwmon_device_register_with_info(dev, "nvme", data,
- &nvme_hwmon_chip_info,
- NULL);
+ hwmon = hwmon_device_register_with_info(dev, "nvme",
+ data, &nvme_hwmon_chip_info,
+ NULL);
if (IS_ERR(hwmon)) {
dev_warn(dev, "Failed to instantiate hwmon device\n");
- devm_kfree(dev, data);
+ kfree(data);
}
-
+ ctrl->hwmon_device = hwmon;
return 0;
}
+
+void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
+{
+ if (ctrl->hwmon_device) {
+ struct nvme_hwmon_data *data =
+ dev_get_drvdata(ctrl->hwmon_device);
+
+ hwmon_device_unregister(ctrl->hwmon_device);
+ ctrl->hwmon_device = NULL;
+ kfree(data);
+ }
+}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 88a6b97247f5..f51b942bb4f5 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -246,6 +246,9 @@ struct nvme_ctrl {
struct rw_semaphore namespaces_rwsem;
struct device ctrl_device;
struct device *device; /* char device */
+#ifdef CONFIG_NVME_HWMON
+ struct device *hwmon_device;
+#endif
struct cdev cdev;
struct work_struct reset_work;
struct work_struct delete_work;
@@ -809,11 +812,14 @@ static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev)
#ifdef CONFIG_NVME_HWMON
int nvme_hwmon_init(struct nvme_ctrl *ctrl);
+void nvme_hwmon_exit(struct nvme_ctrl *ctrl);
#else
static inline int nvme_hwmon_init(struct nvme_ctrl *ctrl)
{
return 0;
}
+
+static inline int nvme_hwmon_exit(struct nvme_ctrl *ctrl) {}
#endif
u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
--
2.29.2
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] nvme/hwmon: rework to avoid devm allocation
2021-01-19 6:22 [PATCH] nvme/hwmon: rework to avoid devm allocation Hannes Reinecke
@ 2021-01-19 6:28 ` Christoph Hellwig
0 siblings, 0 replies; 2+ messages in thread
From: Christoph Hellwig @ 2021-01-19 6:28 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Keith Busch, Daniel Wagner, linux-nvme, Christoph Hellwig,
Guenter Roeck, Sagi Grimberg
As requested last time: please include the author of the code in the
discussion.
On Tue, Jan 19, 2021 at 07:22:42AM +0100, Hannes Reinecke wrote:
> The original design to use device-managed resource allocation
> doesn't really work as the NVMe controller has a vastly different
> lifetime than the hwmon sysfs attributes, causing warning about
> duplicate sysfs entries upon reconnection.
> This patch reworks the hwmon allocation to avoid device-managed
> resource allocation, and uses the NVMe controller as parent for
> the sysfs attributes.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Tested-by: Daniel Wagner <dwagner@suse.de>
> ---
> drivers/nvme/host/core.c | 1 +
> drivers/nvme/host/hwmon.c | 31 +++++++++++++++++++++----------
> drivers/nvme/host/nvme.h | 6 ++++++
> 3 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index fff49e544fdf..3c6c77e44bf7 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4419,6 +4419,7 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
>
> void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
> {
> + nvme_hwmon_exit(ctrl);
> nvme_fault_inject_fini(&ctrl->fault_inject);
> dev_pm_qos_hide_latency_tolerance(ctrl->device);
> cdev_device_del(&ctrl->cdev, ctrl->device);
> diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
> index 552dbc04567b..8f9e96986780 100644
> --- a/drivers/nvme/host/hwmon.c
> +++ b/drivers/nvme/host/hwmon.c
> @@ -223,12 +223,12 @@ static const struct hwmon_chip_info nvme_hwmon_chip_info = {
>
> int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> {
> - struct device *dev = ctrl->dev;
> + struct device *dev = ctrl->device;
> struct nvme_hwmon_data *data;
> struct device *hwmon;
> int err;
>
> - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> if (!data)
> return 0;
>
> @@ -237,19 +237,30 @@ int nvme_hwmon_init(struct nvme_ctrl *ctrl)
>
> err = nvme_hwmon_get_smart_log(data);
> if (err) {
> - dev_warn(ctrl->device,
> - "Failed to read smart log (error %d)\n", err);
> - devm_kfree(dev, data);
> + dev_warn(dev, "Failed to read smart log (error %d)\n", err);
> + kfree(data);
> return err;
> }
>
> - hwmon = devm_hwmon_device_register_with_info(dev, "nvme", data,
> - &nvme_hwmon_chip_info,
> - NULL);
> + hwmon = hwmon_device_register_with_info(dev, "nvme",
> + data, &nvme_hwmon_chip_info,
> + NULL);
> if (IS_ERR(hwmon)) {
> dev_warn(dev, "Failed to instantiate hwmon device\n");
> - devm_kfree(dev, data);
> + kfree(data);
> }
> -
> + ctrl->hwmon_device = hwmon;
> return 0;
> }
> +
> +void nvme_hwmon_exit(struct nvme_ctrl *ctrl)
> +{
> + if (ctrl->hwmon_device) {
> + struct nvme_hwmon_data *data =
> + dev_get_drvdata(ctrl->hwmon_device);
> +
> + hwmon_device_unregister(ctrl->hwmon_device);
> + ctrl->hwmon_device = NULL;
> + kfree(data);
> + }
> +}
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 88a6b97247f5..f51b942bb4f5 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -246,6 +246,9 @@ struct nvme_ctrl {
> struct rw_semaphore namespaces_rwsem;
> struct device ctrl_device;
> struct device *device; /* char device */
> +#ifdef CONFIG_NVME_HWMON
> + struct device *hwmon_device;
> +#endif
> struct cdev cdev;
> struct work_struct reset_work;
> struct work_struct delete_work;
> @@ -809,11 +812,14 @@ static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev)
>
> #ifdef CONFIG_NVME_HWMON
> int nvme_hwmon_init(struct nvme_ctrl *ctrl);
> +void nvme_hwmon_exit(struct nvme_ctrl *ctrl);
> #else
> static inline int nvme_hwmon_init(struct nvme_ctrl *ctrl)
> {
> return 0;
> }
> +
> +static inline int nvme_hwmon_exit(struct nvme_ctrl *ctrl) {}
> #endif
>
> u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> --
> 2.29.2
---end quoted text---
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-19 6:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 6:22 [PATCH] nvme/hwmon: rework to avoid devm allocation Hannes Reinecke
2021-01-19 6:28 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.