Hi Nuno,
On 1/9/24 13:15, Nuno Sá wrote:
On Tue, 2023-12-26 at 15:59 +0000, Jonathan Cameron wrote:
+
+ ret = devm_add_action_or_reset(dev, iio_backend_release, back);
+ if (ret)
+ return ret;
+
+ link = device_link_add(dev, back->dev,
DL_FLAG_AUTOREMOVE_CONSUMER);
+ if (!link)
+ pr_warn("%s: Could not link to supplier(%s)\n",
dev_name(dev),
+ dev_name(back->dev));
Why is that not an error and we try to carry on?
I guess having the links are not really mandatory for the whole thing to
work (more
like a nice to have). That's also how this is handled in another subsystems
so I
figured it would be fine.
But since you are speaking about this... After you pointing me to Bartosz's
talk and
sawing it (as stuff like this is mentioned), I started to question this. The
goal
with the above comment is that if you remove the backend, all the consumers
are
automatically removed (unbound). Just not sure if that's what we always want
(and we
are already handling the situation where a backend goes away with -ENODEV)
as some
frontends could still be useful without the backend (I guess that could be
plausible). I think for now we don't really have such usecases so the links
can make
sense (and we can figure something like optionally creating these links if
we ever
need too) but having your inputs on this will definitely be valuable.
I'm not keen on both trying to make everything tear down cleanly AND making
sure
it all works even if we don't. That just adds two code paths to test when
either
should be sufficient on its own. I don't really mind which. Bartosz's stuff
Agreed...
is nice, but it may not be the right solution here.
There's pros and cons on both options...
For the device links the cons I see is that it depends on patch 3 for it to work
(or some other approach if the one in that patch is not good) - not really a
real con though :). The biggest concern is (possible) future uses where we end
up with cases where removing a backend is not really a "deal breaker". I could
think of frontends that have multiple backends (one per data path) and removing
one backend would not tear the whole thing down (we would just have one non
functional data paths/port where the others are still ok).
Olivier, for STM usecases, do we always need the backend? I mean, does it make
sense to always remove/unbind the frontend in case the backend is unbound?
In STM usecases we may have severals backends linked to a frontend. But
I cannot find a usecase where it may be necessary to remove a backend
while keeping the others alive. So from my side it is acceptable to tear
everythings down if a backend if removed.
Olivier
Maybe some of your usecases already "forces" us with a decision.
The biggest pro I see is code simplicity. If we can assume the frontend can
never exist in case one of the backends is gone, we can:
* get rid of the sync mutex;
* get rid of the kref and bind the backend object lifetime to the backend
device (using devm_kzalloc() instead of kzalloc + refcount.
Basically, we would not need to care about syncing the backend existence with
accessing it...
To sum up, the device_links approach tends to be similar (not identical) to the
previous approach using the component API.
The biggest pro I see in Bartosz's stuff is flexibility. So it should just work
in whatever future usecases we might have. I fear that going the device_links
road we might end up needing this stuff anyways.
Obviously, the biggest con is code complexity (not that bad though) as we always
need to properly sync any backend callback.
- Nuno Sá
