On Wed, 2024-01-10 at 09:16 +0000, Jonathan Cameron wrote:
> On Tue, 09 Jan 2024 13:15:54 +0100
> Nuno Sá <noname.nuno@xxxxxxxxx> 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).
>
> I wouldn't waste time catering to such set ups. If the whole thing gets
> torn down because one element went away that should be fine.
> To do anything else I'd want to see a real world use case.
Fair enough...
>
> >
> > 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?
> >
> > 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.
>
> I'm keen on it if it simplifies code or becomes the dominant paradigm for such
> things in the kernel (so becomes what people expect). That isn't true yet
> and I doubt it will be particularly soon. If things head that way we can
> revisit as it would enable things that currently we don't support - nothing
> should break.
>
Well, If I'm not missing anything, simpler code would be with device_links so I
guess that's your preferred approach for now :). Also fine by me as this is an
in kernel interface so we easily revisit it.
I'll just wait a bit more (before sending v5) to see if Olivier has any
foreseeable usecase where device_links would be an issue.
- Nuno Sá
