Re: [RFC PATCH v1] iio: trigger: move trig->owner init to trigger allocate() stage

[Dmitry Rokosov]

Hello Jonathan,

Thank you for comments. I have a several questions about the flow,
please find them below.

On Sat, Jun 04, 2022 at 02:59:55PM +0100, Jonathan Cameron wrote:
> On Wed, 1 Jun 2022 17:48:32 +0000
> Dmitry Rokosov <DDRokosov@xxxxxxxxxxxxxx> wrote:
> 
> > To provide a new IIO trigger to the IIO core, usually driver executes the
> > following pipeline: allocate()/register()/get(). Before, IIO core assigned
> > trig->owner as a pointer to the module which registered this trigger at
> > the register() stage. But actually the trigger object is owned by the
> > module earlier, on the allocate() stage, when trigger object is
> > successfully allocated for the driver.
> > 
> > This patch moves trig->owner initialization from register()
> > stage of trigger initialization pipeline to allocate() stage to
> > eliminate all misunderstandings and time gaps between trigger object
> > creation and owner acquiring.
> > 
> > Signed-off-by: Dmitry Rokosov <ddrokosov@xxxxxxxxxxxxxx>
> 
> Hi Dmitry,
> 
> I 'think' this is fine, but its in the high risk category that I'd like
> to keep it on list for a few weeks before applying.
> 

Could you please explain what it means? Do you have some testing branch
with such dangerous patches or do we need just to wait other developers
for more points of view? Thanks in advance.

> Note I'm still keen that in general we keep the flow such that
> we do allocate()/register()/get() as there is no guarantee that the get()
> will never do anything that requires the trigger to be registered, even
> though that is true today.  Which is another way of saying I'm still
> keen we fix up any cases that sneak in after your fix up set dealt with
> the current ones.

I fully agree with you. I suppose to resolve such a problem we need to
have some indicators that the trigger is already registered or not.
>From my point of view, trig->list entry fits well to answer this question.
Trigger is added to the global IIO triggers list during register()
execution, so we can just check that entry is not empty to make sure that
trigger is registered.

I've sent a v2 patch version, where I use trig->list entry empty status to
warn it:

https://lore.kernel.org/linux-iio/20220606111316.19265-1-ddrokosov@xxxxxxxxxxxxxx/

> 
> Thanks for following up on this!
> 
> Jonathan
> 

-- 
Thank you,
Dmitry