Re: [RFC PATCH 20/27] net/intel: write descriptors using non-volatile pointers

Bruce Richardson
2025-12-22T09:50:28Z

On Sat, Dec 20, 2025 at 09:43:46AM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Friday, 19 December 2025 18.26
> >
> > Use a non-volatile uint64_t pointer to store to the descriptor ring.
> > This will allow the compiler to optionally merge the stores as it sees
> > best.
>
> I suppose there was a reason for the volatile.
> Is removing it really safe?
> E.g. this will also allow the compiler to reorder stores; not just the pair of 64-bits, but also stores to multiple descriptors.
>

It would be more risky to remove for reads than for writes, I believe,
since when reading we have the possibility of the NIC doing stores to the
descriptors at the same time. In the case of writing new descriptors we
know that the NIC will never read the descriptors until such time as we hit
the doorball/write the tail value. Therefore, so long as we have a fence
before the tail write, (or as part of the tail write), it doesn't matter
what actual order the descriptor stores hit the ring.

> One more comment inline below.
>
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > drivers/net/intel/common/tx_scalar_fns.h | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/intel/common/tx_scalar_fns.h
> > b/drivers/net/intel/common/tx_scalar_fns.h
> > index 7b643fcf44..95e9acbe60 100644
> > --- a/drivers/net/intel/common/tx_scalar_fns.h
> > +++ b/drivers/net/intel/common/tx_scalar_fns.h
> > @@ -184,6 +184,15 @@ struct ci_timesstamp_queue_fns {
> > write_ts_tail_t write_ts_tail;
> > };
> >
> > +static inline void
> > +write_txd(volatile void *txd, uint64_t qw0, uint64_t qw1)
> > +{
> > + uint64_t *txd_qw = RTE_CAST_PTR(void *, txd);
>
> If the descriptors are 16-byte aligned, you could mark them as such, so the compiler can use 128-bit stores on architectures where alignment matters.
>
Sure, I can try adding an aligned tag to this.

/Bruce