Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Mohamed Ayman <mohamedaymanworkspace@gmail.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@arm.com>,
	James Clark <james.clark@linaro.org>, Leo Yan <leo.yan@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
	<coresight@lists.linaro.org>,
	"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
	<linux-arm-kernel@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT"
	<linux-rt-devel@lists.linux.dev>
Subject: Re: [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref()
Date: Tue, 14 Jul 2026 12:42:52 +0200	[thread overview]
Message-ID: <20260714104252.NuPnAQyJ@linutronix.de> (raw)
In-Reply-To: <20260713230028.8046-1-mohamedaymanworkspace@gmail.com>

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@gmail.com>
> ---
> @@ -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


  reply	other threads:[~2026-07-14 10:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
2026-07-13  3:07 ` Jie Gan
2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
2026-07-14 10:42   ` Sebastian Andrzej Siewior [this message]
2026-07-14 19:42     ` MOHAMED AYMAN
2026-07-15  6:58       ` Sebastian Andrzej Siewior
2026-07-16  3:07         ` MOHAMED AYMAN
2026-07-16 12:32           ` Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260714104252.NuPnAQyJ@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=clrkwllms@kernel.org \
    --cc=coresight@lists.linaro.org \
    --cc=james.clark@linaro.org \
    --cc=leo.yan@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=mike.leach@arm.com \
    --cc=mohamedaymanworkspace@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox