On Sat, Dec 20, 2025 at 09:43:46AM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:[email protected]] > > 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 <[email protected]> > > --- > > 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
