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.
>
> 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.
Jonathan
>
> Obviously, the biggest con is code complexity (not that bad though) as we always
> need to properly sync any backend callback.
>
> - Nuno Sá
> >
>
