RDMA: Use unsigned comparison in CQ cleanup loops

[PATCH rdma-next 0/4] RDMA: Use unsigned comparison in CQ cleanup loops

Posted by Edward Srouji 1 week, 2 days ago

mlx5, mlx4, mthca and hns all sweep the CQ backwards when cleaning
completions for a QP that is being destroyed, using the same open-coded
loop (mlx5 shown):
	while ((int) --prod_index - (int) cq->mcq.cons_index >= 0)
Both indexes are free running u32 counters, so the comparison has to be
done modulo 2^32. Casting each operand to int and subtracting does not
do that: the subtraction overflows once the two indexes straddle 2^31,
which is undefined behaviour, and a compiler that assumes signed
overflow cannot occur is free to fold the expression into a plain signed
comparison that is not wraparound safe.
This is not a fix. The kernel is built with -fno-strict-overflow, so
gcc and clang both retain the subtraction, the generated code is
unaffected, and there is no known user-visible impact.  The casts buy
nothing, though, and the correctness of these loops should not rest on
a build flag. The series drops the arithmetic instead:
	while (prod_index != cq->mcq.cons_index) {
		--prod_index;
		...
No functional change intended.

Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
Yishai Hadas (4):
      RDMA/mlx5: Use unsigned comparison in the CQ cleanup loop
      RDMA/mlx4: Use unsigned comparison in the CQ cleanup loop
      RDMA/mthca: Use unsigned comparison in the CQ cleanup loop
      RDMA/hns: Use unsigned comparison in the CQ cleanup loop

 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 3 ++-
 drivers/infiniband/hw/mlx4/cq.c            | 3 ++-
 drivers/infiniband/hw/mlx5/cq.c            | 3 ++-
 drivers/infiniband/hw/mthca/mthca_cq.c     | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)
---
base-commit: 3e1de7f906ab162b23d6fe0eabccf687a98fa25f
change-id: 20260915-fix-cq-cleanup-6512d4346ed7

Best regards,
-- 
Edward Srouji <edwards@nvidia.com>

Re: [PATCH rdma-next 0/4] RDMA: Use unsigned comparison in CQ cleanup loops

Posted by Leon Romanovsky 2 days, 17 hours ago

On Tue, 15 Sep 2026 18:33:27 +0300, Edward Srouji wrote:
> mlx5, mlx4, mthca and hns all sweep the CQ backwards when cleaning
> completions for a QP that is being destroyed, using the same open-coded
> loop (mlx5 shown):
> 	while ((int) --prod_index - (int) cq->mcq.cons_index >= 0)
> Both indexes are free running u32 counters, so the comparison has to be
> done modulo 2^32. Casting each operand to int and subtracting does not
> do that: the subtraction overflows once the two indexes straddle 2^31,
> which is undefined behaviour, and a compiler that assumes signed
> overflow cannot occur is free to fold the expression into a plain signed
> comparison that is not wraparound safe.
> This is not a fix. The kernel is built with -fno-strict-overflow, so
> gcc and clang both retain the subtraction, the generated code is
> unaffected, and there is no known user-visible impact.  The casts buy
> nothing, though, and the correctness of these loops should not rest on
> a build flag. The series drops the arithmetic instead:
> 	while (prod_index != cq->mcq.cons_index) {
> 		--prod_index;
> 		...
> No functional change intended.
> 
> [...]

Applied, thanks!

[1/4] RDMA/mlx5: Use unsigned comparison in the CQ cleanup loop
      https://git.kernel.org/rdma/rdma/c/c6aa5feb521066
[2/4] RDMA/mlx4: Use unsigned comparison in the CQ cleanup loop
      https://git.kernel.org/rdma/rdma/c/c5fe428bb23edc
[3/4] RDMA/mthca: Use unsigned comparison in the CQ cleanup loop
      https://git.kernel.org/rdma/rdma/c/b934024d3aafb9
[4/4] RDMA/hns: Use unsigned comparison in the CQ cleanup loop
      https://git.kernel.org/rdma/rdma/c/ebb65190349537

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>