Re: [PATCH v1 1/2] ACPI / utils: Introduce acpi_dev_get_first_device() helper

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

 



Hi,

On 12-07-18 19:23, Andy Shevchenko wrote:
acpi_dev_present() and acpi_dev_get_first_match_name() are missing
put_device() call and thus keeping reference counting unbalanced.

In order to fix the issue introduce a new helper to convert existing users
one-by-one to a better API.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
  drivers/acpi/utils.c    | 50 ++++++++++++++++++++++++-----------------
  include/acpi/acpi_bus.h |  2 ++
  include/linux/acpi.h    |  6 +++++
  3 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 78db97687f26..b54651b3d4bd 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -738,7 +738,6 @@ bool acpi_dev_found(const char *hid)
  EXPORT_SYMBOL(acpi_dev_found);
struct acpi_dev_match_info {
-	const char *dev_name;
  	struct acpi_device_id hid[2];
  	const char *uid;
  	s64 hrv;
@@ -758,8 +757,6 @@ static int acpi_dev_match_cb(struct device *dev, void *data)
  	    strcmp(adev->pnp.unique_id, match->uid)))
  		return 0;
- match->dev_name = acpi_dev_name(adev);
-
  	if (match->hrv == -1)
  		return 1;
@@ -771,18 +768,18 @@ static int acpi_dev_match_cb(struct device *dev, void *data)
  }
/**
- * acpi_dev_present - Detect that a given ACPI device is present
+ * acpi_dev_get_first_match - Return a first match of ACPI device if present
   * @hid: Hardware ID of the device.
   * @uid: Unique ID of the device, pass NULL to not check _UID
   * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
   *
- * Return %true if a matching device was present at the moment of invocation.
- * Note that if the device is pluggable, it may since have disappeared.
+ * Return a pointer to the first matching ACPI device.
+ * Caller must put device back to balance reference counting.
   *
   * Note that unlike acpi_dev_found() this function checks the status
- * of the device. So for devices which are present in the dsdt, but
+ * of the device. So for devices which are present in the DSDT, but
   * which are disabled (their _STA callback returns 0) this function
- * will return false.
+ * will return NULL.
   *
   * For this function to work, acpi_bus_scan() must have been executed
   * which happens in the subsys_initcall() subsection. Hence, do not
@@ -790,7 +787,8 @@ static int acpi_dev_match_cb(struct device *dev, void *data)
   * instead). Calling from module_init() is fine (which is synonymous
   * with device_initcall()).
   */
-bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
+struct acpi_device *
+acpi_dev_get_first_match(const char *hid, const char *uid, s64 hrv)
  {
  	struct acpi_dev_match_info match = {};
  	struct device *dev;
@@ -800,7 +798,25 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
  	match.hrv = hrv;
dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
-	return !!dev;
+	return dev ? to_acpi_device(dev) : NULL;
+}
+EXPORT_SYMBOL(acpi_dev_get_first_match);
+
+/**
+ * acpi_dev_present - Detect that a given ACPI device is present
+ * @hid: Hardware ID of the device.
+ * @uid: Unique ID of the device, pass NULL to not check _UID
+ * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
+ *
+ * DEPRECATED, use acpi_dev_get_first_match() directly!
+ *
+ * Return %true if a matching device is present.
+ */
+bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
+{
+	struct acpi_device *adev = acpi_dev_get_first_match(hid, uid, hrv);
+
+	return !!adev;
  }
  EXPORT_SYMBOL(acpi_dev_present);

Why not just do:

bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
{
	struct acpi_device *adev = acpi_dev_get_first_match(hid, uid, hrv);

	if (!adev)
		return false;

	put_device(&adev->dev);
	return true;
}

And not deprecate this. This fixes the leak while keeping the
API usage simple for users of this API. Having to do a put_device
in all callers seems cumbersome if we can just do it here.

Regards,

Hans

@@ -810,23 +826,17 @@ EXPORT_SYMBOL(acpi_dev_present);
   * @uid: Unique ID of the device, pass NULL to not check _UID
   * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
   *
+ * DEPRECATED, use acpi_dev_get_first_match() directly!
+ *
   * Return device name if a matching device was present
   * at the moment of invocation, or NULL otherwise.
- *
- * See additional information in acpi_dev_present() as well.
   */
  const char *
  acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv)
  {
-	struct acpi_dev_match_info match = {};
-	struct device *dev;
-
-	strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id));
-	match.uid = uid;
-	match.hrv = hrv;
+	struct acpi_device *adev = acpi_dev_get_first_match(hid, uid, hrv);
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
-	return dev ? match.dev_name : NULL;
+	return adev ? acpi_dev_name(adev) : NULL;
  }
  EXPORT_SYMBOL(acpi_dev_get_first_match_name);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ba4dd54f2c82..53ca4403f772 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -91,6 +91,8 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
  bool acpi_dev_found(const char *hid);
  bool acpi_dev_present(const char *hid, const char *uid, s64 hrv);
+struct acpi_device *
+acpi_dev_get_first_match(const char *hid, const char *uid, s64 hrv);
  const char *
  acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index e54f40974eb0..098e0af003b4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -657,6 +657,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
  	return false;
  }
+struct acpi_device *
+acpi_dev_get_first_match(const char *hid, const char *uid, s64 hrv)
+{
+	return NULL;
+}
+
  static inline const char *
  acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv)
  {

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux