[PATCH v3] libcxgb: use raw_cpu_ptr() in ppm_get_cpu_entries()

[Haoran Wang]

ppm_get_cpu_entries() disables preemption with get_cpu(), reads the
current CPU's pool with per_cpu_ptr(), then acquires the pool's
spin_lock_bh() while preemption is still disabled, and finally put_cpu().

On PREEMPT_RT kernels spin_lock_bh() becomes a sleeping lock
(rt_spin_lock), so the task may sleep with preempt_count elevated:
scheduling while atomic, reported as a BUG by CONFIG_DEBUG_ATOMIC_SLEEP
and a deadlock once the lock is contended.  Any iSCSI offload I/O
through a Chelsio NIC that reaches ppm_get_cpu_entries() triggers it.

Read the current CPU with raw_cpu_ptr() without raising preempt_count,
and derive the global index base from pool->base (initialised when the
pools are allocated) instead of cpu * pool_index_max, so the base
always matches the pool that was locked.  The owning CPU remains in the
debug output, now derived from pool->base.

Signed-off-by: Haoran Wang <haoranwangsec@xxxxxxxxx>

---
(Resent with the complete Cc list; the earlier copy reached only you.)

Hi Sebastian,

thanks for the review.  v3 below addresses your comments:

- The commit message now describes what the patch does; the review
  rationale moved into the changelog below the --- separator.
- PREEMPT_RT spelled out.
- pr_debug() keeps printing the owning CPU of the pool, derived as
  pool->base / ppm->pool_index_max; the global index stays
  i + pool->base.
- Tested on: x86_64, compile-tested with CONFIG_PREEMPT_RT=y and
  CONFIG_DEBUG_ATOMIC_SLEEP=y; no Chelsio hardware at hand, so the
  change is not exercised on real offload I/O.

Changes since v2:
- Keep the owning CPU in the pr_debug() output, derived from
  pool->base / ppm->pool_index_max (suggested by Sebastian).
- Reword the commit message to describe what the patch does and move
  the review discussion into this changelog; spell PREEMPT_RT.

Changes since v1:
- Drop the cpu variable: read the global-index base from pool->base
  (initialised at pool allocation) instead of a cpu captured before
  the lock, so the base is always consistent with the pool that was
  locked.  The per-CPU pools are shards of the global ppod index
  space and not tied to the running CPU (ppm_unmark_entries() derives
  the owning pool from the index itself), so locking another CPU's
  pool after a migration is a cache-locality matter, not a
  correctness one.

 drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

--- a/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c
+++ b/drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c
@@ -119,16 +119,13 @@
 			       unsigned long caller_data)
 {
 	struct cxgbi_ppm_pool *pool;
-	unsigned int cpu;
 	int i;
 
 	if (!ppm->pool)
 		return -EINVAL;
 
-	cpu = get_cpu();
-	pool = per_cpu_ptr(ppm->pool, cpu);
+	pool = raw_cpu_ptr(ppm->pool);
 	spin_lock_bh(&pool->lock);
-	put_cpu();
 
 	i = ppm_find_unused_entries(pool->bmap, ppm->pool_index_max,
 				    pool->next, count, 0);
@@ -145,10 +142,10 @@
 	spin_unlock_bh(&pool->lock);
 
 	pr_debug("%s: cpu %u, idx %d + %d (%d), next %u.\n",
-		 __func__, cpu, i, count, i + cpu * ppm->pool_index_max,
-		pool->next);
+		 __func__, pool->base / ppm->pool_index_max, i, count,
+		i + pool->base, pool->next);
 
-	i += cpu * ppm->pool_index_max;
+	i += pool->base;
 	ppm_mark_entries(ppm, i, count, caller_data);
 
 	return i;
@@ -375,6 +372,7 @@
 
 		memset(ppool, 0, alloc_sz);
 		spin_lock_init(&ppool->lock);
+		ppool->base = count;
 		count += ppmax;
 	}