lkml.org 
[lkml]   [2022]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] driver core: Fix repeated device_is_dependent check for same link
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
>

\
 
 \ /
  Last update: 2022-07-05 16:10    [W:0.033 / U:0.848 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site