Re: [PATCH v4 5/7] remoteproc: core: set recovery_disabled when doing rproc_add()

[Dmitry Baryshkov]

On Wed, Mar 11, 2026 at 01:39:50AM -0700, Bartosz Golaszewski wrote:
> On Wed, 11 Mar 2026 03:11:42 +0100, Dmitry Baryshkov
> <dmitry.baryshkov@xxxxxxxxxxxxxxxx> said:
> > On Tue, Mar 10, 2026 at 06:50:30AM -0700, Bartosz Golaszewski wrote:
> >>
> >> Ideally things like this would be passed to the rproc core in some kind of a
> >> config structure and only set when registration succeeds. This looks to me
> >> like papering over the real issue and I think it's still racy as there's no
> >> true synchronization.
> >>
> >> Wouldn't it be better to take rproc->lock for the entire duration of
> >> rproc_add()? It's already initialized in rproc_alloc().
> >
> > It would still be racy as rproc_trigger_recovery() is called outside of
> > the lock. Instead the error cleanup path (and BTW, rproc_del() path too)
> > must explicitly call cancel_work_sync() on the crash_handler work (and
> > any other work items that can be scheduled).
> >
> 
> This looks weird TBH. For example: rproc_crash_handler_work() takes the lock,
> but releases it right before calling inspecting rproc->recovery_disabled and
> calling rproc_trigger_recovery(). It looks wrong, I think it should keep the
> lock and rptoc_trigger_recovery() should enforce it being taken before the
> call.

Yes. Nevertheless the driver should cancel the work too.

-- 
With best wishes
Dmitry