Re: [for-next 02/12] RDMA/irdma: Fix data race on cqp_request->request_done

[Leon Romanovsky <leon@kernel.org>]

On Tue, Mar 17, 2026 at 07:27:47PM +0000, Nikolova, Tatyana E wrote:
> 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Tuesday, March 17, 2026 8:23 AM
> > To: Czurylo, Krzysztof <krzysztof.czurylo@intel.com>
> > Cc: jgg@nvidia.com; linux-rdma@vger.kernel.org; Nikolova, Tatyana E
> > <tatyana.e.nikolova@intel.com>
> > Subject: Re: [for-next 02/12] RDMA/irdma: Fix data race on cqp_request-
> > >request_done
> > 
> > On Tue, Mar 17, 2026 at 12:14:21PM +0000, Czurylo, Krzysztof wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Leon Romanovsky <leon@kernel.org>
> > > > Sent: 17 March, 2026 12:13
> > > > To: Nikolova, Tatyana E <tatyana.e.nikolova@intel.com>
> > > > Cc: jgg@nvidia.com; linux-rdma@vger.kernel.org; Czurylo, Krzysztof
> > > > <krzysztof.czurylo@intel.com>
> > > > Subject: Re: [for-next 02/12] RDMA/irdma: Fix data race on cqp_request-
> > > > >request_done
> > > >
> > > > On Mon, Mar 16, 2026 at 01:39:39PM -0500, Tatyana Nikolova wrote:
> > > > > From: Krzysztof Czurylo <krzysztof.czurylo@intel.com>
> > > > >
> > > > > Changes type of request_done flag from bool to atomic_t to fix
> > > > > data race in irdma_complete_cqp_request / irdma_wait_event
> > > > > reported by KCSAN:
> > > >
> > > > Again, this fix is wrong too.
> > >
> > > Could you please elaborate on what is wrong with this fix?
> > > And/or suggest how to fix it properly?
> > >
> > > Please note 'request_done' is _not_ a bitfield and we only do simple
> > > load/store operations on it.  There is no RMW.
> > > Despite this, KCSAN still reports a data race on it.
> > >
> > > Honestly, the original idea was just to change the type from
> > > 'bool' to 'u8'.  This is enough to silence KCSAN, but it is
> > > not clear why.  Perhaps it indicates a bug in KCSAN?
> > 
> > Yes, both u8 and atomic_t behave the same, they can't be interrupted
> > during read/write. This is why KCSAN doesn't warn you.
> > 
> > > I mean, maybe the report below is a false positive?
> > 
> > Sounds like that.
> 
> Leon,
> 
> Are you okay taking the rest of the patches in the series (they apply without the two KCSAN related patches) or you prefer that I resubmit the series?

Sure, let's me to pick them now.

> 
> For this specific patch, are you okay with dropping all atomic changes, but making request_done u8 (currently bool) to silence the KCSAN warning?

In general, I don't like changing code just to silence a tool, but in this
case it is justified. Switching to u8 not only quiets the warning, it also
reduces the size of struct irdma_cqp_request, just run pahole on it.

Thanks