diff mbox series

[v3,01/10] ACPI / utils: Introduce acpi_dev_get_first_match_dev() helper

Message ID 20190328171729.44002-2-andriy.shevchenko@linux.intel.com (mailing list archive)
State Accepted
Commit 817b4d64da036f5559297a2fdb82b8b14f4ffdcd
Headers show
Series ACPI / utils: Replace leaky function | expand

Commit Message

Andy Shevchenko March 28, 2019, 5:17 p.m. UTC 
The acpi_dev_get_first_match_name() is 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. And then remove the old one.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/utils.c    | 24 ++++++++++++++++++++++--
 include/acpi/acpi_bus.h |  3 +++
 include/linux/acpi.h    |  6 ++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki March 28, 2019, 8:12 p.m. UTC | #1 
On Thu, Mar 28, 2019 at 6:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The acpi_dev_get_first_match_name() is 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. And then remove the old one.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Any differences between this and the previous one?  I've queued up
that one already.

> ---
>  drivers/acpi/utils.c    | 24 ++++++++++++++++++++++--
>  include/acpi/acpi_bus.h |  3 +++
>  include/linux/acpi.h    |  6 ++++++
>  3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index c4b06cc075f9..5a2bae2b6c3a 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -739,6 +739,7 @@ EXPORT_SYMBOL(acpi_dev_found);
>
>  struct acpi_dev_match_info {
>         const char *dev_name;
> +       struct acpi_device *adev;
>         struct acpi_device_id hid[2];
>         const char *uid;
>         s64 hrv;
> @@ -759,6 +760,7 @@ static int acpi_dev_match_cb(struct device *dev, void *data)
>                 return 0;
>
>         match->dev_name = acpi_dev_name(adev);
> +       match->adev = adev;
>
>         if (match->hrv == -1)
>                 return 1;
> @@ -806,16 +808,34 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>  EXPORT_SYMBOL(acpi_dev_present);
>
>  /**
> - * acpi_dev_get_first_match_name - Return name of first match of ACPI device
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
>   * @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 device name if a matching device was present
> + * Return the first match of ACPI device if a matching device was present
>   * at the moment of invocation, or NULL otherwise.
>   *
> + * The caller is responsible to call put_device() on the returned device.
> + *
>   * See additional information in acpi_dev_present() as well.
>   */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(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;
> +
> +       dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> +       return dev ? match.adev : NULL;
> +}
> +EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
> +
> +/* DEPRECATED, use acpi_dev_get_first_match_dev() instead */
>  const char *
>  acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv)
>  {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 0300374101cd..2063e9e2f384 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -91,6 +91,9 @@ 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_dev(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 d5dcebd7aad3..3e1d16b00513 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -669,6 +669,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>         return false;
>  }
>
> +static inline struct acpi_device *
> +acpi_dev_get_first_match_dev(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)
>  {
> --
> 2.20.1
>
Andy Shevchenko March 28, 2019, 9:38 p.m. UTC | #2 
On Thu, Mar 28, 2019 at 09:12:49PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 28, 2019 at 6:17 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > The acpi_dev_get_first_match_name() is 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. And then remove the old one.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Any differences between this and the previous one?  I've queued up
> that one already.

Not a single bit. Just Hans' tag is added.

> 
> > ---
> >  drivers/acpi/utils.c    | 24 ++++++++++++++++++++++--
> >  include/acpi/acpi_bus.h |  3 +++
> >  include/linux/acpi.h    |  6 ++++++
> >  3 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> > index c4b06cc075f9..5a2bae2b6c3a 100644
> > --- a/drivers/acpi/utils.c
> > +++ b/drivers/acpi/utils.c
> > @@ -739,6 +739,7 @@ EXPORT_SYMBOL(acpi_dev_found);
> >
> >  struct acpi_dev_match_info {
> >         const char *dev_name;
> > +       struct acpi_device *adev;
> >         struct acpi_device_id hid[2];
> >         const char *uid;
> >         s64 hrv;
> > @@ -759,6 +760,7 @@ static int acpi_dev_match_cb(struct device *dev, void *data)
> >                 return 0;
> >
> >         match->dev_name = acpi_dev_name(adev);
> > +       match->adev = adev;
> >
> >         if (match->hrv == -1)
> >                 return 1;
> > @@ -806,16 +808,34 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
> >  EXPORT_SYMBOL(acpi_dev_present);
> >
> >  /**
> > - * acpi_dev_get_first_match_name - Return name of first match of ACPI device
> > + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> >   * @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 device name if a matching device was present
> > + * Return the first match of ACPI device if a matching device was present
> >   * at the moment of invocation, or NULL otherwise.
> >   *
> > + * The caller is responsible to call put_device() on the returned device.
> > + *
> >   * See additional information in acpi_dev_present() as well.
> >   */
> > +struct acpi_device *
> > +acpi_dev_get_first_match_dev(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;
> > +
> > +       dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> > +       return dev ? match.adev : NULL;
> > +}
> > +EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
> > +
> > +/* DEPRECATED, use acpi_dev_get_first_match_dev() instead */
> >  const char *
> >  acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv)
> >  {
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index 0300374101cd..2063e9e2f384 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -91,6 +91,9 @@ 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_dev(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 d5dcebd7aad3..3e1d16b00513 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -669,6 +669,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
> >         return false;
> >  }
> >
> > +static inline struct acpi_device *
> > +acpi_dev_get_first_match_dev(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)
> >  {
> > --
> > 2.20.1
> >
Mark Brown April 1, 2019, 7:23 a.m. UTC | #3 
On Thu, Mar 28, 2019 at 07:17:20PM +0200, Andy Shevchenko wrote:
> The acpi_dev_get_first_match_name() is 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. And then remove the old one.

Acked-by: Mark Brown <broonie@kernel.org>
diff mbox series

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index c4b06cc075f9..5a2bae2b6c3a 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -739,6 +739,7 @@  EXPORT_SYMBOL(acpi_dev_found);
 
 struct acpi_dev_match_info {
 	const char *dev_name;
+	struct acpi_device *adev;
 	struct acpi_device_id hid[2];
 	const char *uid;
 	s64 hrv;
@@ -759,6 +760,7 @@  static int acpi_dev_match_cb(struct device *dev, void *data)
 		return 0;
 
 	match->dev_name = acpi_dev_name(adev);
+	match->adev = adev;
 
 	if (match->hrv == -1)
 		return 1;
@@ -806,16 +808,34 @@  bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
 EXPORT_SYMBOL(acpi_dev_present);
 
 /**
- * acpi_dev_get_first_match_name - Return name of first match of ACPI device
+ * acpi_dev_get_first_match_dev - Return the first match of ACPI device
  * @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 device name if a matching device was present
+ * Return the first match of ACPI device if a matching device was present
  * at the moment of invocation, or NULL otherwise.
  *
+ * The caller is responsible to call put_device() on the returned device.
+ *
  * See additional information in acpi_dev_present() as well.
  */
+struct acpi_device *
+acpi_dev_get_first_match_dev(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;
+
+	dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
+	return dev ? match.adev : NULL;
+}
+EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
+
+/* DEPRECATED, use acpi_dev_get_first_match_dev() instead */
 const char *
 acpi_dev_get_first_match_name(const char *hid, const char *uid, s64 hrv)
 {
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 0300374101cd..2063e9e2f384 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -91,6 +91,9 @@  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_dev(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 d5dcebd7aad3..3e1d16b00513 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -669,6 +669,12 @@  static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
 	return false;
 }
 
+static inline struct acpi_device *
+acpi_dev_get_first_match_dev(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)
 {