RE: [PATCH v4 7/7] bbdev: add a lock option for enqueue/dequeue operation

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

Hi Stephen, Tom., 

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> On Wed, 6 Jul 2022 12:01:19 -0700
> Tom Rix <trix@redhat.com> wrote:
> 
> > On 7/5/22 5:23 PM, Nicolas Chautru wrote:
> > > Locking is not explicitly required but can be valuable in case the
> > > application cannot guarantee to be thread-safe, or specifically is
> > > at risk of using the same queue from multiple threads.
> > > This is an option for PMD to use this.
> > >
> > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > > ---
> > >   lib/bbdev/rte_bbdev.h | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/bbdev/rte_bbdev.h b/lib/bbdev/rte_bbdev.h index
> > > b7ecf94..8e7ca86 100644
> > > --- a/lib/bbdev/rte_bbdev.h
> > > +++ b/lib/bbdev/rte_bbdev.h
> > > @@ -407,6 +407,8 @@ struct rte_bbdev_queue_data {
> > >   	struct rte_bbdev_stats queue_stats;  /**< Queue statistics */
> > >   	enum rte_bbdev_enqueue_status enqueue_status; /**< Enqueue
> status when op is rejected */
> > >   	bool started;  /**< Queue state */
> > > +	rte_rwlock_t lock_enq; /**< lock protection for the Enqueue */
> > > +	rte_rwlock_t lock_deq; /**< lock protection for the Dequeue */
> >
> > No.
> >
> > This is a good idea but needs a use before introducing another
> > element, particularly a complicated one like locking
> >
> > Tom

The actual usage would be implemented within the PMD. Basically this to prevent the corner case when a queue is being accessed from multiple thread for which there is no protection in DPDK (but application does not necessarily behaves well). 
In normal operation there would never be a case when there is a conflict on the lock.
This is not something which was considered for any other PMD?
From DPDK doc : "If multiple threads are to use the same hardware queue on the same NIC port, then locking, or some other form of mutual exclusion, is necessary."
Basically for AC100 we would purely enforce the lock for any enqueue/dequeue operation for a given queue (distinct lock for enqueue and dequeue, since these would run on different threads).  

> Having two locks on same cacheline will create lots of ping/pong false sharing.

You would recommend to purely spread them within the structure? Or something else? 
 
> Also, unless the reader is holding the lock for a significant fraction of the time a
> regular spin lock will be faster.

OK Thanks. It should in principle never have to wait for the lock for the usage above, only to catch misbehaving application risk. 

Nic