Hi Gavin, Thanks for your comments. Your suggestions are applicable in this case. I will remove the usage of rte_wmb() and rte_rmb() in all places as they are not required. I will add rte_smp_wmb() before pending count update in enqueue operation and add rte_smp_rmb() before reading softreq from pending queue. I will add rte_io_wmb() before the ring doorbell to ensure all the DMA buffers & descriptors stores are completed. We check command completion based on the completion code update (which is the last word in the output buffer) and hence rte_io_rmb() is not required.
Best Regards, Dheeraj > -----Original Message----- > From: Gavin Hu (Arm Technology China) <gavin...@arm.com> > Sent: Saturday, September 28, 2019 8:17 PM > To: Nagadheeraj Rottela <rnagadhee...@marvell.com>; > akhil.go...@nxp.com; pablo.de.lara.gua...@intel.com > Cc: Srikanth Jampala <jsrika...@marvell.com>; dev@dpdk.org; Honnappa > Nagarahalli <honnappa.nagaraha...@arm.com>; Gavin Hu (Arm Technology > China) <gavin...@arm.com>; nd <n...@arm.com>; nd <n...@arm.com> > Subject: [EXT] RE: [PATCH v6 0/8] add Nitrox crypto device support > > External Email > > ---------------------------------------------------------------------- > Hi Nagadheeraj, > > I am no expert in crypto dev, maybe you can educate me if I am wrong: > I got an impression in this series, the barriers were used too much, too > heavily and unnecessarily. > > For enqueue operations, I understand they are stores to the DMA buffer, > the queue will be fetched and updated by the crypto device after processing, > then dequeued by the other CPU cores. So for enqueue operations, an > rte_io_wmb is required before the doorbell ringing, and an rte_smp_wmb is > required to ensure the enqueue operations were done before the consumer > on the other side(who dequeues) sees the updated pending_count. For > dequeue operations, rte_smp_rmb is required after reading the > pending_count to ensure reading the intact content from the queue(if the > queue entries were not handled yet by the crypto dev, the status will show > that, maybe an rte_io_rmb is required to ensure the status is read out first). > > The rte_smp_xmb can even be optimized with C11 atomics, but it can be > next step. > > Best Regards, > Gavin
