Re: [PATCH v4 5/7] remoteproc: core: set recovery_disabled when doing rproc_add()
From: Bartosz Golaszewski
Date: Wed Mar 11 2026 - 04:41:35 EST
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.
Bart