On 7/6/22 1:21 PM, Chautru, Nicolas wrote:
Hi Stephen, Tom.,

-----Original Message-----
From: Stephen Hemminger <step...@networkplumber.org>

On Wed, 6 Jul 2022 12:01:19 -0700
Tom Rix <t...@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.chau...@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).

I am fine with locking, just have to use them.

For me, this would mean adding them to every public interface so the changes would be involved.

This is a big change and if pressed to get this patchset into 22.11, then defer this patch to later.

Tom


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



Reply via email to