Re: [PATCH 5/5] hwmon: (drivetemp) Use devres function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/21/21 6:01 PM, Jackie Liu wrote:
From: Jackie Liu <liuyun01@xxxxxxxxxx>

Use devm_hwmon_device_register_with_info() and remove hwmon_dev
from drivetemp_data struct as it is not needed anymore.

Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx>
---
  drivers/hwmon/drivetemp.c | 15 +++++++--------
  1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
index 1eb37106a220..1a5a62288c0a 100644
--- a/drivers/hwmon/drivetemp.c
+++ b/drivers/hwmon/drivetemp.c
@@ -113,7 +113,6 @@ struct drivetemp_data {
  	struct mutex lock;		/* protect data buffer accesses */
  	struct scsi_device *sdev;	/* SCSI device */
  	struct device *dev;		/* instantiating device */
-	struct device *hwdev;		/* hardware monitoring device */
  	u8 smartdata[ATA_SECT_SIZE];	/* local buffer */
  	int (*get_temp)(struct drivetemp_data *st, u32 attr, long *val);
  	bool have_temp_lowest;		/* lowest temp in SCT status */
@@ -555,6 +554,7 @@ static int drivetemp_add(struct device *dev, struct class_interface *intf)
  {
  	struct scsi_device *sdev = to_scsi_device(dev->parent);
  	struct drivetemp_data *st;
+	struct device *hwmon_dev;
  	int err;
st = kzalloc(sizeof(*st), GFP_KERNEL);
@@ -570,13 +570,13 @@ static int drivetemp_add(struct device *dev, struct class_interface *intf)
  		goto abort;
  	}
- st->hwdev = hwmon_device_register_with_info(dev->parent, "drivetemp",
-						    st, &drivetemp_chip_info,
-						    NULL);
-	if (IS_ERR(st->hwdev)) {
-		err = PTR_ERR(st->hwdev);
+	hwmon_dev =
+		devm_hwmon_device_register_with_info(dev->parent, "drivetemp",
+						     st, &drivetemp_chip_info,
+						     NULL);
+	err = PTR_ERR_OR_ZERO(hwmon_dev);
+	if (err)
  		goto abort;
-	}
list_add(&st->list, &drivetemp_devlist);
  	return 0;
@@ -593,7 +593,6 @@ static void drivetemp_remove(struct device *dev, struct class_interface *intf)
  	list_for_each_entry_safe(st, tmp, &drivetemp_devlist, list) {
  		if (st->dev == dev) {
  			list_del(&st->list);
-			hwmon_device_unregister(st->hwdev);
  			kfree(st);
  			break;
  		}


With this change in place, hwmon devices are removed _after_ struct drivetemp_data
is released, which means that there is a race condition where hwmon access is still
possible and uses a released data structure. Besides, it is not well defined if
the lifetime of the hwmon device matches the lifetime of the parent device. I don't
think you know what you are doing, sorry, and I am not even going to look into the
other patches of this series.

Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux