Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()

[MOHAMED AYMAN]

Hi Sebastian,

Thank you for the review and the feedback.

Regarding the commit message:
You are completely right. I will update the wording in the v4 patch to
explicitly state that it "uses a spinlock_t for locking which becomes
a sleeping lock on PREEMPT_RT" instead of calling it an rt_mutex
directly.

Regarding the module-ref counter:
We don't explicitly bump the module reference count for each new
device. Instead, we rely on `destroy_workqueue(coresight_wq)` inside
`coresight_exit()`. `destroy_workqueue()` synchronously drains all
pending work items before returning, which ensures no deferred puts
are executed after the module is unmapped.

Regarding deferring coresight_device_release() vs put_device():
My initial v1 patch did exactly what you suggested, it only deferred
the body of `coresight_device_release()`. However, it was pointed out
that `put_device()` synchronously recurses into `kobject_cleanup()`,
which invokes the child's release function and immediately afterwards
calls `kobject_put(parent)`.

If we only defer the child's release callback, the `put_device()` call
itself will still execute in the atomic CPU_PM notifier context. If
the parent device's release path acquires any sleeping locks, we will
still hit a "scheduling while atomic" panic. Deferring `put_device()`
entirely protects against this parent cascade.

If it is strictly guaranteed that Coresight parent devices (liike
AMBA) will never sleep during their release paths, I can happily
revert to the simpler approach of just deferring
`coresight_device_release()`.

Would you prefer I revert to deferring just the release function, or
keep the current architecture to safeguard the parent put?

Best regards,,
Mohamed Ayman

On Tue, Jul 14, 2026 at 1:42 PM Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
>
> On 2026-07-14 02:00:27 [+0300], Mohamed Ayman wrote:
> > Dropping the last reference to a coresight_device triggers a kernel panic
> > on PREEMPT_RT builds due to a "scheduling while atomic" violation.
> >
> > During CPU idle transitions, coresight_cpu_pm_notify() runs with
> > interrupts disabled. It eventually calls put_device(), which can
> > synchronously trigger the device's release callback and drop the parent
> > device's reference. On PREEMPT_RT, free_percpu() takes a sleeping lock
> > (rt-mutex), and the parent's release callback might also sleep. Sleeping
>
> It is a spinlock_t which we refer as a sleeping lock. There is "struct
> rt_mutex" which is somehow different.
> I would suggest to word it like "uses a spinlock_t for locking which
> becomes a sleeping lock on PREEMPT_RT".
>
> > in this atomic PM context crashes the system.
> >
> > A previous patch tried deferring just the coresight_device_release() body,
> > but this still left the synchronous put_device() call dangerously exposed
> > to sleeping parent release functions.
> >
> > Fix this by entirely deferring the put_device() call to process context.
> > We add a pending counter (put_pending) and a work_struct to the coresight
> > device. When releasing a reference, we increment the counter and queue
> > the work. A worker thread then safely drains the counter and calls
> > put_device(). The counter prevents leaking references if multiple puts
> > are queued before the worker even has a chance to run.
> >
> > To prevent a use-after-free race condition during module unload, the work
> > is queued on a dedicated coresight_wq which is safely drained and
> > destroyed in coresight_exit().
>
> Do you have anything that keeps the module-ref counter up with each new
> device?
>
> > Finally, remove the unnecessary raw_spinlock_irqsave in the put path,
> > as dropping a reference doesn't require protecting the per-CPU table.
> >
> > Signed-off-by: Mohamed Ayman <mohamedaymanworkspace@xxxxxxxxx>
> > ---
> …
> > @@ -163,16 +175,9 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev)
> >       if (!csdev || !coresight_is_percpu_source(csdev))
> >               return;
> >
> > -     guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> > +     atomic_inc(&csdev->put_pending);
> >
> > -     /*
> > -      * TODO: coresight_device_release() is invoked to release resources when
> > -      * the device's refcount reaches zero. It then calls free_percpu(),
> > -      * which acquires pcpu_lock — a sleepable lock when PREEMPT_RT is
> > -      * enabled. Since the raw spinlock coresight_dev_lock is held, this can
> > -      * lead to a potential "scheduling while atomic" issue.
> > -      */
> > -     put_device(&csdev->dev);
> > +     queue_work(coresight_wq, &csdev->put_work);
>
> What about you keep this as-is and just delay coresight_device_release()
> instead?
>
> >  }
> >
> >  struct coresight_device *coresight_get_source(struct coresight_path *path)
>
> Sebastian