Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
Sebastian Andrzej Siewior <[email protected]>
| Newsgroups | dev.linux.lists.linux-rt-devel,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> > --- … > @@ -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