Messages in this thread | | | From | Saravana Kannan <> | Date | Wed, 29 Sep 2021 12:01:07 -0700 | Subject | Re: [PATCH v1] driver core: Reject pointless SYNC_STATE_ONLY device links |
| |
On Wed, Sep 29, 2021 at 5:12 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Sep 29, 2021 at 2:51 AM 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> > > --- > > drivers/base/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index 15986cc2fe5e..eed27933ac4d 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -716,7 +716,7 @@ struct device_link *device_link_add(struct device *consumer, > > * SYNC_STATE_ONLY link, we don't check for reverse dependencies > > * because it only affects sync_state() callbacks. > > */ > > - if (!device_pm_initialized(supplier) > > + if (!device_pm_initialized(supplier) || consumer == supplier > > Why do we need to get all the way down to here in order to return NULL > in the consumer == supplier case?
This is where it used to be checked before the "Fixes commit" so I added it back here. But sure, I can move it up there.
-Saravana
> > IMO this should be checked at the beginning along with !consumer and !supplier. > > > || (!(flags & DL_FLAG_SYNC_STATE_ONLY) && > > device_is_dependent(consumer, supplier))) { > > link = NULL; > > -- > > 2.33.0.685.g46640cef36-goog > >
| |