* [RFC] driver core: Fix repeated device_is_dependent check for same link
@ 2022-07-05 13:45 Abel Vesa
2022-07-05 13:56 ` Abel Vesa
0 siblings, 1 reply; 2+ messages in thread
From: Abel Vesa @ 2022-07-05 13:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Saravana Kannan,
Thomas Gleixner
Cc: Linux Kernel Mailing List, linux-arm-msm, Abel Vesa
In case of a cyclic dependency, if the supplier is not yet available,
the parent of the supplier is checked for dependency. But if there are
more than one suppliers with the same parent, the first check returns
true while the next checks skip that specific link entirely. So add a
flag that marks the link for future checks and bail early if it is
already marked with that flag.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
For more details about this issue, have a look at this thread:
https://lore.kernel.org/all/CAGETcx8F0wP+RA0KpjOJeZfc=DVG-MbM_=SkRHD4UhD2ReL7Kw@mail.gmail.com/
drivers/base/core.c | 14 +++++++++++---
include/linux/device.h | 2 ++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ccdd5b4295de..38cb478ae400 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -297,12 +297,18 @@ int device_is_dependent(struct device *dev, void *target)
return ret;
list_for_each_entry(link, &dev->links.consumers, s_node) {
+ /* if already marked before as dependent, bail early */
+ if (link->flags & DL_FLAG_DEVICE_IS_DEPENDENT)
+ return 1;
+
if ((link->flags & ~DL_FLAG_INFERRED) ==
(DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED))
continue;
- if (link->consumer == target)
+ if (link->consumer == target) {
+ link->flags |= DL_FLAG_DEVICE_IS_DEPENDENT;
return 1;
+ }
ret = device_is_dependent(link->consumer, target);
if (ret)
@@ -1660,11 +1666,13 @@ static void fw_devlink_relax_link(struct device_link *link)
if (!(link->flags & DL_FLAG_INFERRED))
return;
- if (link->flags == (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE))
+ if ((link->flags & (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) ==
+ (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE))
return;
pm_runtime_drop_link(link);
- link->flags = DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE;
+ link->flags &= DL_FLAG_DEVICE_IS_DEPENDENT;
+ link->flags |= DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE;
dev_dbg(link->consumer, "Relaxing link with %s\n",
dev_name(link->supplier));
}
diff --git a/include/linux/device.h b/include/linux/device.h
index 424b55df0272..3b0c4b777a60 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -317,6 +317,7 @@ enum device_link_state {
* MANAGED: The core tracks presence of supplier/consumer drivers (internal).
* SYNC_STATE_ONLY: Link only affects sync_state() behavior.
* INFERRED: Inferred from data (eg: firmware) and not from driver actions.
+ * DEVICE_IS_DEPENDENT: The consumer is dependent on the supplier
*/
#define DL_FLAG_STATELESS BIT(0)
#define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1)
@@ -327,6 +328,7 @@ enum device_link_state {
#define DL_FLAG_MANAGED BIT(6)
#define DL_FLAG_SYNC_STATE_ONLY BIT(7)
#define DL_FLAG_INFERRED BIT(8)
+#define DL_FLAG_DEVICE_IS_DEPENDENT BIT(9)
/**
* enum dl_dev_state - Device driver presence tracking information.
--
2.34.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC] driver core: Fix repeated device_is_dependent check for same link
2022-07-05 13:45 [RFC] driver core: Fix repeated device_is_dependent check for same link Abel Vesa
@ 2022-07-05 13:56 ` Abel Vesa
0 siblings, 0 replies; 2+ messages in thread
From: Abel Vesa @ 2022-07-05 13:56 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Saravana Kannan,
Thomas Gleixner
Cc: Linux Kernel Mailing List, linux-arm-msm
On 22-07-05 16:45:02, Abel Vesa wrote:
> In case of a cyclic dependency, if the supplier is not yet available,
> the parent of the supplier is checked for dependency. But if there are
> more than one suppliers with the same parent, the first check returns
> true while the next checks skip that specific link entirely. So add a
> flag that marks the link for future checks and bail early if it is
> already marked with that flag.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>
> For more details about this issue, have a look at this thread:
> https://lore.kernel.org/all/CAGETcx8F0wP+RA0KpjOJeZfc=DVG-MbM_=SkRHD4UhD2ReL7Kw@mail.gmail.com/
>
> drivers/base/core.c | 14 +++++++++++---
> include/linux/device.h | 2 ++
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index ccdd5b4295de..38cb478ae400 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -297,12 +297,18 @@ int device_is_dependent(struct device *dev, void *target)
> return ret;
>
> list_for_each_entry(link, &dev->links.consumers, s_node) {
> + /* if already marked before as dependent, bail early */
> + if (link->flags & DL_FLAG_DEVICE_IS_DEPENDENT)
> + return 1;
On a second thought, this is wrong since it might be a different
consumer link.
> +
> if ((link->flags & ~DL_FLAG_INFERRED) ==
> (DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED))
> continue;
>
> - if (link->consumer == target)
> + if (link->consumer == target) {
> + link->flags |= DL_FLAG_DEVICE_IS_DEPENDENT;
> return 1;
> + }
>
> ret = device_is_dependent(link->consumer, target);
> if (ret)
> @@ -1660,11 +1666,13 @@ static void fw_devlink_relax_link(struct device_link *link)
> if (!(link->flags & DL_FLAG_INFERRED))
> return;
>
> - if (link->flags == (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE))
> + if ((link->flags & (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) ==
> + (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE))
> return;
>
> pm_runtime_drop_link(link);
> - link->flags = DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE;
> + link->flags &= DL_FLAG_DEVICE_IS_DEPENDENT;
> + link->flags |= DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE;
> dev_dbg(link->consumer, "Relaxing link with %s\n",
> dev_name(link->supplier));
> }
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 424b55df0272..3b0c4b777a60 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -317,6 +317,7 @@ enum device_link_state {
> * MANAGED: The core tracks presence of supplier/consumer drivers (internal).
> * SYNC_STATE_ONLY: Link only affects sync_state() behavior.
> * INFERRED: Inferred from data (eg: firmware) and not from driver actions.
> + * DEVICE_IS_DEPENDENT: The consumer is dependent on the supplier
> */
> #define DL_FLAG_STATELESS BIT(0)
> #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1)
> @@ -327,6 +328,7 @@ enum device_link_state {
> #define DL_FLAG_MANAGED BIT(6)
> #define DL_FLAG_SYNC_STATE_ONLY BIT(7)
> #define DL_FLAG_INFERRED BIT(8)
> +#define DL_FLAG_DEVICE_IS_DEPENDENT BIT(9)
>
> /**
> * enum dl_dev_state - Device driver presence tracking information.
> --
> 2.34.3
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-05 14:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-05 13:45 [RFC] driver core: Fix repeated device_is_dependent check for same link Abel Vesa
2022-07-05 13:56 ` Abel Vesa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox