Re: [for-next 03/12] RDMA/irdma: Change ah_valid type to atomic

[Leon Romanovsky <leon@kernel.org>]

On Mon, Mar 16, 2026 at 01:39:40PM -0500, Tatyana Nikolova wrote:
> From: Krzysztof Czurylo <krzysztof.czurylo@intel.com>
> 
> Converts ah_valid flag to atomic to protect it against data-race.

Atomic operations don't prevent data races; they only guarantee that a
read or write happens atomically. Use the "xxx yyy:1" bitfield construct
only for fields that cannot be modified concurrently.

Thanks

> 
> [  809.561229] BUG: KCSAN: data-race in irdma_create_hw_ah [irdma] / irdma_gsi_ud_qp_ah_cb [irdma]
> 
> [  809.565182] write to 0xffff8d911c8eee73 of 1 bytes by task 38949 on cpu 10:
> [  809.567113]  irdma_gsi_ud_qp_ah_cb+0x41/0x60 [irdma]
> [  809.567343]  irdma_complete_cqp_request+0x44/0xb0 [irdma]
> [  809.567572]  irdma_cqp_ce_handler+0x242/0x290 [irdma]
> [  809.567810]  irdma_wait_event+0xf2/0x3c0 [irdma]
> 
> [  809.570951] read to 0xffff8d911c8eee73 of 1 bytes by task 38952 on cpu 7:
> [  809.573037]  irdma_create_hw_ah+0x1e7/0x370 [irdma]
> [  809.573265]  irdma_create_ah+0x61/0x70 [irdma]
> [  809.573491]  _rdma_create_ah+0x262/0x290 [ib_core]
> [  809.573831]  rdma_create_ah+0xa3/0x140 [ib_core]
> 
> [  809.576933] value changed: 0x06 -> 0x07
> [  809.581184] Reported by Kernel Concurrency Sanitizer
> 
> Fixes: dd90451fac23 ("RDMA/irdma: Add RoCEv2 UD OP support")
> Signed-off-by: Krzysztof Czurylo <krzysztof.czurylo@intel.com>
> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com>
> ---
>  drivers/infiniband/hw/irdma/cm.c    |  2 +-
>  drivers/infiniband/hw/irdma/puda.c  |  2 +-
>  drivers/infiniband/hw/irdma/uda.h   |  2 +-
>  drivers/infiniband/hw/irdma/utils.c | 16 +++++++++-------
>  drivers/infiniband/hw/irdma/verbs.c |  7 ++++---
>  5 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/cm.c b/drivers/infiniband/hw/irdma/cm.c
> index 3d084d4ff577..947fd93f5fb0 100644
> --- a/drivers/infiniband/hw/irdma/cm.c
> +++ b/drivers/infiniband/hw/irdma/cm.c
> @@ -313,7 +313,7 @@ static struct irdma_puda_buf *irdma_form_ah_cm_frame(struct irdma_cm_node *cm_no
>  	u32 pd_len = 0;
>  	u32 hdr_len = 0;
>  
> -	if (!cm_node->ah || !cm_node->ah->ah_info.ah_valid) {
> +	if (!cm_node->ah || !atomic_read(&cm_node->ah->ah_info.ah_valid)) {
>  		ibdev_dbg(&cm_node->iwdev->ibdev, "CM: AH invalid\n");
>  		return NULL;
>  	}
> diff --git a/drivers/infiniband/hw/irdma/puda.c b/drivers/infiniband/hw/irdma/puda.c
> index 4f1a8c97faf1..3410be38f602 100644
> --- a/drivers/infiniband/hw/irdma/puda.c
> +++ b/drivers/infiniband/hw/irdma/puda.c
> @@ -1652,7 +1652,7 @@ static void irdma_ieq_handle_exception(struct irdma_puda_rsrc *ieq,
>  	}
>  	if (hw_rev == IRDMA_GEN_1)
>  		irdma_ieq_process_fpdus(qp, ieq);
> -	else if (pfpdu->ah && pfpdu->ah->ah_info.ah_valid)
> +	else if (pfpdu->ah && atomic_read(&pfpdu->ah->ah_info.ah_valid))
>  		irdma_ieq_process_fpdus(qp, ieq);
>  exit:
>  	spin_unlock_irqrestore(&pfpdu->lock, flags);
> diff --git a/drivers/infiniband/hw/irdma/uda.h b/drivers/infiniband/hw/irdma/uda.h
> index 27b8701cf21b..773c33ce62a2 100644
> --- a/drivers/infiniband/hw/irdma/uda.h
> +++ b/drivers/infiniband/hw/irdma/uda.h
> @@ -22,7 +22,7 @@ struct irdma_ah_info {
>  	u8 tc_tos;
>  	u8 hop_ttl;
>  	u8 mac_addr[ETH_ALEN];
> -	bool ah_valid:1;
> +	atomic_t ah_valid;
>  	bool ipv4_valid:1;
>  	bool do_lpbk:1;
>  };
> diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
> index f9c99c216a2c..be2dc92d008f 100644
> --- a/drivers/infiniband/hw/irdma/utils.c
> +++ b/drivers/infiniband/hw/irdma/utils.c
> @@ -1967,7 +1967,8 @@ int irdma_ah_cqp_op(struct irdma_pci_f *rf, struct irdma_sc_ah *sc_ah, u8 cmd,
>  		return -ENOMEM;
>  
>  	if (wait)
> -		sc_ah->ah_info.ah_valid = (cmd == IRDMA_OP_AH_CREATE);
> +		atomic_set(&sc_ah->ah_info.ah_valid,
> +			   (cmd == IRDMA_OP_AH_CREATE));
>  
>  	return 0;
>  }
> @@ -1984,10 +1985,10 @@ static void irdma_ieq_ah_cb(struct irdma_cqp_request *cqp_request)
>  
>  	spin_lock_irqsave(&qp->pfpdu.lock, flags);
>  	if (!cqp_request->compl_info.op_ret_val) {
> -		sc_ah->ah_info.ah_valid = true;
> +		atomic_set(&sc_ah->ah_info.ah_valid, true);
>  		irdma_ieq_process_fpdus(qp, qp->vsi->ieq);
>  	} else {
> -		sc_ah->ah_info.ah_valid = false;
> +		atomic_set(&sc_ah->ah_info.ah_valid, false);
>  		irdma_ieq_cleanup_qp(qp->vsi->ieq, qp);
>  	}
>  	spin_unlock_irqrestore(&qp->pfpdu.lock, flags);
> @@ -2002,7 +2003,8 @@ static void irdma_ilq_ah_cb(struct irdma_cqp_request *cqp_request)
>  	struct irdma_cm_node *cm_node = cqp_request->param;
>  	struct irdma_sc_ah *sc_ah = cm_node->ah;
>  
> -	sc_ah->ah_info.ah_valid = !cqp_request->compl_info.op_ret_val;
> +	atomic_set(&sc_ah->ah_info.ah_valid,
> +		   !cqp_request->compl_info.op_ret_val);
>  	irdma_add_conn_est_qh(cm_node);
>  }
>  
> @@ -2069,7 +2071,7 @@ void irdma_puda_free_ah(struct irdma_sc_dev *dev, struct irdma_sc_ah *ah)
>  	if (!ah)
>  		return;
>  
> -	if (ah->ah_info.ah_valid) {
> +	if (atomic_read(&ah->ah_info.ah_valid)) {
>  		irdma_ah_cqp_op(rf, ah, IRDMA_OP_AH_DESTROY, false, NULL, NULL);
>  		irdma_free_rsrc(rf, rf->allocated_ahs, ah->ah_info.ah_idx);
>  	}
> @@ -2086,9 +2088,9 @@ void irdma_gsi_ud_qp_ah_cb(struct irdma_cqp_request *cqp_request)
>  	struct irdma_sc_ah *sc_ah = cqp_request->param;
>  
>  	if (!cqp_request->compl_info.op_ret_val)
> -		sc_ah->ah_info.ah_valid = true;
> +		atomic_set(&sc_ah->ah_info.ah_valid, true);
>  	else
> -		sc_ah->ah_info.ah_valid = false;
> +		atomic_set(&sc_ah->ah_info.ah_valid, false);
>  }
>  
>  /**
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 38bc0e656ecf..9cfcdf7b053e 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -5116,8 +5116,8 @@ static int irdma_create_hw_ah(struct irdma_device *iwdev, struct irdma_ah *ah, b
>  
>  		if (poll_timeout_us_atomic(irdma_cqp_ce_handler(rf,
>  								&rf->ccq.sc_cq),
> -					   ah->sc_ah.ah_info.ah_valid, 1,
> -					   tmout_ms * USEC_PER_MSEC, false)) {
> +					   atomic_read(&ah->sc_ah.ah_info.ah_valid),
> +					   1, tmout_ms * USEC_PER_MSEC, false)) {
>  			ibdev_dbg(&iwdev->ibdev,
>  				  "VERBS: CQP create AH timed out");
>  			err = -ETIMEDOUT;
> @@ -5236,7 +5236,8 @@ static bool irdma_ah_exists(struct irdma_device *iwdev,
>  	hash_for_each_possible(iwdev->rf->ah_hash_tbl, ah, list, key) {
>  		/* Set ah_valid and ah_id the same so memcmp can work */
>  		new_ah->sc_ah.ah_info.ah_idx = ah->sc_ah.ah_info.ah_idx;
> -		new_ah->sc_ah.ah_info.ah_valid = ah->sc_ah.ah_info.ah_valid;
> +		atomic_set(&new_ah->sc_ah.ah_info.ah_valid,
> +			   atomic_read(&ah->sc_ah.ah_info.ah_valid));
>  		if (!memcmp(&ah->sc_ah.ah_info, &new_ah->sc_ah.ah_info,
>  			    sizeof(ah->sc_ah.ah_info))) {
>  			refcount_inc(&ah->refcnt);
> -- 
> 2.31.1
>