LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links
@ 2021-09-29 19:05 Saravana Kannan
  2021-09-29 19:24 ` Rafael J. Wysocki
  2021-09-30 12:04 ` Ulf Hansson
  0 siblings, 2 replies; 3+ messages in thread
From: Saravana Kannan @ 2021-09-29 19:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Saravana Kannan
  Cc: Ulf Hansson, kernel-team, linux-kernel

SYNC_STATE_ONLY device links intentionally allow cycles because cyclic
sync_state() dependencies are valid and necessary.

However a SYNC_STATE_ONLY device link where the consumer and the supplier
are the same device is pointless because the device link would be deleted
as soon as the device probes (because it's also the consumer) and won't
affect when the sync_state() callback is called. It's a waste of CPU cycles
and memory to create this device link. So reject any attempts to create
such a device link.

Fixes: 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag")
Reported-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
v1 -> v2:
- Moved the check higher up in the function.

 drivers/base/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 15986cc2fe5e..249da496581a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -687,7 +687,8 @@ struct device_link *device_link_add(struct device *consumer,
 {
 	struct device_link *link;
 
-	if (!consumer || !supplier || flags & ~DL_ADD_VALID_FLAGS ||
+	if (!consumer || !supplier || consumer == supplier ||
+	    flags & ~DL_ADD_VALID_FLAGS ||
 	    (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) ||
 	    (flags & DL_FLAG_SYNC_STATE_ONLY &&
 	     (flags & ~DL_FLAG_INFERRED) != DL_FLAG_SYNC_STATE_ONLY) ||
-- 
2.33.0.685.g46640cef36-goog


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links
  2021-09-29 19:05 [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links Saravana Kannan
@ 2021-09-29 19:24 ` Rafael J. Wysocki
  2021-09-30 12:04 ` Ulf Hansson
  1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2021-09-29 19:24 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Ulf Hansson,
	Cc: Android Kernel, Linux Kernel Mailing List

On Wed, Sep 29, 2021 at 9:05 PM Saravana Kannan <saravanak@google.com> wrote:
>
> SYNC_STATE_ONLY device links intentionally allow cycles because cyclic
> sync_state() dependencies are valid and necessary.
>
> However a SYNC_STATE_ONLY device link where the consumer and the supplier
> are the same device is pointless because the device link would be deleted
> as soon as the device probes (because it's also the consumer) and won't
> affect when the sync_state() callback is called. It's a waste of CPU cycles
> and memory to create this device link. So reject any attempts to create
> such a device link.
>
> Fixes: 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag")
> Reported-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> v1 -> v2:
> - Moved the check higher up in the function.
>
>  drivers/base/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 15986cc2fe5e..249da496581a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -687,7 +687,8 @@ struct device_link *device_link_add(struct device *consumer,
>  {
>         struct device_link *link;
>
> -       if (!consumer || !supplier || flags & ~DL_ADD_VALID_FLAGS ||
> +       if (!consumer || !supplier || consumer == supplier ||
> +           flags & ~DL_ADD_VALID_FLAGS ||
>             (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) ||
>             (flags & DL_FLAG_SYNC_STATE_ONLY &&
>              (flags & ~DL_FLAG_INFERRED) != DL_FLAG_SYNC_STATE_ONLY) ||
> --
> 2.33.0.685.g46640cef36-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links
  2021-09-29 19:05 [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links Saravana Kannan
  2021-09-29 19:24 ` Rafael J. Wysocki
@ 2021-09-30 12:04 ` Ulf Hansson
  1 sibling, 0 replies; 3+ messages in thread
From: Ulf Hansson @ 2021-09-30 12:04 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Android Kernel Team,
	Linux Kernel Mailing List

On Wed, 29 Sept 2021 at 21:05, Saravana Kannan <saravanak@google.com> wrote:
>
> SYNC_STATE_ONLY device links intentionally allow cycles because cyclic
> sync_state() dependencies are valid and necessary.
>
> However a SYNC_STATE_ONLY device link where the consumer and the supplier
> are the same device is pointless because the device link would be deleted
> as soon as the device probes (because it's also the consumer) and won't
> affect when the sync_state() callback is called. It's a waste of CPU cycles
> and memory to create this device link. So reject any attempts to create
> such a device link.
>
> Fixes: 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag")
> Reported-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
> v1 -> v2:
> - Moved the check higher up in the function.
>
>  drivers/base/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 15986cc2fe5e..249da496581a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -687,7 +687,8 @@ struct device_link *device_link_add(struct device *consumer,
>  {
>         struct device_link *link;
>
> -       if (!consumer || !supplier || flags & ~DL_ADD_VALID_FLAGS ||
> +       if (!consumer || !supplier || consumer == supplier ||
> +           flags & ~DL_ADD_VALID_FLAGS ||
>             (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) ||
>             (flags & DL_FLAG_SYNC_STATE_ONLY &&
>              (flags & ~DL_FLAG_INFERRED) != DL_FLAG_SYNC_STATE_ONLY) ||
> --
> 2.33.0.685.g46640cef36-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-30 12:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 19:05 [PATCH v2] driver core: Reject pointless SYNC_STATE_ONLY device links Saravana Kannan
2021-09-29 19:24 ` Rafael J. Wysocki
2021-09-30 12:04 ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).