Re: [PATCH v4] libcxgb: use raw_cpu_ptr() in ppm_get_cpu_entries()

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-11-10 16:57:51 [+0800], Haoran Wang wrote:
> ppm_get_cpu_entries() must not acquire a sleeping lock while preemption
> is disabled: get_cpu() raises preempt_count and spin_lock_bh() may sleep

I would say "disables preemption" rather than "raises preempt_count".

> on PREEMPT_RT kernels.  Read the current CPU with raw_cpu_ptr() instead,
> which keeps the per-CPU accessor stable without disabling preemption.

That is not what is happening. raw_cpu_ptr() obtains the per-CPU
pointer of ppm->pool of the current CPU. I don't know what means
"accessor stable" and the following. Without disabled preemption the
scheduler can migrate the task to a different CPU. The access is still
stable but if the "locking" of the data structure is based on "the
data structure can only be accessed from the local CPU" then it is no
longer valid.

> Using the pool of a CPU the thread has since migrated away from is
> harmless: the per-CPU pools are shards of the global ppod index space,

ppod?

> and the free path ppm_unmark_entries() derives the owning pool from the
> index instead of from the running CPU, so any thread can service any
> shard.  Derive the global index base from pool->base so it always
> matches the pool that was locked.

The important part is that ppm_find_unused_entries() acquires slots
(read-modify-write operation on the structure) and it happens under the
lock therefore cross-CPU access does not matter. Does it always happen
under the lock? Then it is safe - otherwise it is not.

> Signed-off-by: Haoran Wang <haoranwangsec@gmail.com>
> 
> 
> thanks for the comments.  v4 below:
> 
> - pr_debug() now labels the pool's owner as "pool cpu" and also prints
>   the running CPU, so a thread servicing a foreign shard is visible in
>   the log.

Was this asked for? The pool CPU might be interesting to know so you
have which CPU's pool is serving the request. But the current CPU is
probably less useful.

The changes below look reasonable and I *think* it is unchanged from the
last time I looked at them

Please look at Documentation/process/maintainer-netdev.rst for the
subject prefix, fixes tag and "Resending after review".

Sebastian