DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
@ 2024-02-22 16:21 Maxime Coquelin
  2024-02-22 16:21 ` [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin
  2024-02-22 18:05 ` [PATCH 1/2] baseband/fpga_5gnr_fec: " Chautru, Nicolas
  0 siblings, 2 replies; 6+ messages in thread
From: Maxime Coquelin @ 2024-02-22 16:21 UTC (permalink / raw)
  To: dev, nicolas.chautru, hernan.vargas, david.marchand, thomas, roretzla
  Cc: Maxime Coquelin

rte_smp_rmb() is deprecated, use the new API instead as
suggested in rte_atomic header.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
index efc1d3a772..314c87350e 100644
--- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
+++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
@@ -2661,7 +2661,7 @@ vc_5gnr_dequeue_ldpc_enc_one_op_cb(struct fpga_5gnr_queue *q, struct rte_bbdev_e
 		return -1;
 
 	/* make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	rte_bbdev_log_debug("DMA response desc %p", desc);
 
@@ -2690,7 +2690,7 @@ agx100_dequeue_ldpc_enc_one_op_cb(struct fpga_5gnr_queue *q, struct rte_bbdev_en
 		return -1;
 
 	/* make sure the response is read atomically. */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	rte_bbdev_log_debug("DMA response desc %p", desc);
 
@@ -2722,7 +2722,7 @@ vc_5gnr_dequeue_ldpc_dec_one_op_cb(struct fpga_5gnr_queue *q, struct rte_bbdev_d
 		return -1;
 
 	/* make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	vc_5gnr_print_dma_dec_desc_debug_info(desc);
@@ -2768,7 +2768,7 @@ agx100_dequeue_ldpc_dec_one_op_cb(struct fpga_5gnr_queue *q, struct rte_bbdev_de
 		return -1;
 
 	/* make sure the response is read atomically. */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	agx100_print_dma_dec_desc_debug_info(desc);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] baseband/fpga_lte_fec: use new barrier API
  2024-02-22 16:21 [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API Maxime Coquelin
@ 2024-02-22 16:21 ` Maxime Coquelin
  2024-02-22 18:05 ` [PATCH 1/2] baseband/fpga_5gnr_fec: " Chautru, Nicolas
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2024-02-22 16:21 UTC (permalink / raw)
  To: dev, nicolas.chautru, hernan.vargas, david.marchand, thomas, roretzla
  Cc: Maxime Coquelin

rte_smp_rmb() is deprecated, use the new API instead as
suggested in rte_atomic header.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
index a4a963f74d..4af969fab8 100644
--- a/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
+++ b/drivers/baseband/fpga_lte_fec/fpga_lte_fec.c
@@ -2091,7 +2091,7 @@ dequeue_enc_one_op_cb(struct fpga_queue *q, struct rte_bbdev_enc_op **op,
 		return -1;
 
 	/* make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	rte_bbdev_log_debug("DMA response desc %p", desc);
 
@@ -2121,7 +2121,7 @@ dequeue_enc_one_op_tb(struct fpga_queue *q, struct rte_bbdev_enc_op **op,
 		return -1;
 
 	/* Make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	/* Verify if done bit in all CBs is set */
 	cbs_in_op = desc->enc_req.cbs_in_op;
@@ -2133,7 +2133,7 @@ dequeue_enc_one_op_tb(struct fpga_queue *q, struct rte_bbdev_enc_op **op,
 	}
 
 	/* Make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	for (cb_idx = 0; cb_idx < cbs_in_op; ++cb_idx) {
 		desc = q->ring_addr + ((q->head_free_desc + desc_offset +
@@ -2164,7 +2164,7 @@ dequeue_dec_one_op_cb(struct fpga_queue *q, struct rte_bbdev_dec_op **op,
 		return -1;
 
 	/* make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	print_dma_dec_desc_debug_info(desc);
@@ -2199,7 +2199,7 @@ dequeue_dec_one_op_tb(struct fpga_queue *q, struct rte_bbdev_dec_op **op,
 		return -1;
 
 	/* Make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	/* Verify if done bit in all CBs is set */
 	cbs_in_op = desc->dec_req.cbs_in_op;
@@ -2211,7 +2211,7 @@ dequeue_dec_one_op_tb(struct fpga_queue *q, struct rte_bbdev_dec_op **op,
 	}
 
 	/* Make sure the response is read atomically */
-	rte_smp_rmb();
+	rte_atomic_thread_fence(rte_memory_order_acquire);
 
 	for (cb_idx = 0; cb_idx < cbs_in_op; ++cb_idx) {
 		desc = q->ring_addr + ((q->head_free_desc + desc_offset +
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
  2024-02-22 16:21 [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API Maxime Coquelin
  2024-02-22 16:21 ` [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin
@ 2024-02-22 18:05 ` Chautru, Nicolas
  2024-02-26 11:03   ` Maxime Coquelin
  1 sibling, 1 reply; 6+ messages in thread
From: Chautru, Nicolas @ 2024-02-22 18:05 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Vargas, Hernan, Marchand, David, thomas, roretzla

Hi Maxime, 

Why would we change this here and now? Is the intent not to use new suggested semantics for new patches only?
Are all DPDK drivers being changed?
I am unsure we would want to change these drivers, this is kind of risk introduced by code churn that gets ecosystem unwilling to move to latest version.
These memory barriers issues are awful to troubleshoot or properly validate, so personally quite reluctant to change. 

Thanks
Nic

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, February 22, 2024 8:21 AM
> To: dev@dpdk.org; Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas,
> Hernan <hernan.vargas@intel.com>; Marchand, David
> <david.marchand@redhat.com>; thomas@monjalon.net;
> roretzla@linux.microsoft.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
> 
> rte_smp_rmb() is deprecated, use the new API instead as suggested in
> rte_atomic header.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> index efc1d3a772..314c87350e 100644
> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> @@ -2661,7 +2661,7 @@ vc_5gnr_dequeue_ldpc_enc_one_op_cb(struct
> fpga_5gnr_queue *q, struct rte_bbdev_e
>  		return -1;
> 
>  	/* make sure the response is read atomically */
> -	rte_smp_rmb();
> +	rte_atomic_thread_fence(rte_memory_order_acquire);
> 
>  	rte_bbdev_log_debug("DMA response desc %p", desc);
> 
> @@ -2690,7 +2690,7 @@ agx100_dequeue_ldpc_enc_one_op_cb(struct
> fpga_5gnr_queue *q, struct rte_bbdev_en
>  		return -1;
> 
>  	/* make sure the response is read atomically. */
> -	rte_smp_rmb();
> +	rte_atomic_thread_fence(rte_memory_order_acquire);
> 
>  	rte_bbdev_log_debug("DMA response desc %p", desc);
> 
> @@ -2722,7 +2722,7 @@ vc_5gnr_dequeue_ldpc_dec_one_op_cb(struct
> fpga_5gnr_queue *q, struct rte_bbdev_d
>  		return -1;
> 
>  	/* make sure the response is read atomically */
> -	rte_smp_rmb();
> +	rte_atomic_thread_fence(rte_memory_order_acquire);
> 
>  #ifdef RTE_LIBRTE_BBDEV_DEBUG
>  	vc_5gnr_print_dma_dec_desc_debug_info(desc);
> @@ -2768,7 +2768,7 @@ agx100_dequeue_ldpc_dec_one_op_cb(struct
> fpga_5gnr_queue *q, struct rte_bbdev_de
>  		return -1;
> 
>  	/* make sure the response is read atomically. */
> -	rte_smp_rmb();
> +	rte_atomic_thread_fence(rte_memory_order_acquire);
> 
>  #ifdef RTE_LIBRTE_BBDEV_DEBUG
>  	agx100_print_dma_dec_desc_debug_info(desc);
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
  2024-02-22 18:05 ` [PATCH 1/2] baseband/fpga_5gnr_fec: " Chautru, Nicolas
@ 2024-02-26 11:03   ` Maxime Coquelin
  2024-02-27 16:56     ` Maxime Coquelin
  2024-08-12  9:58     ` Maxime Coquelin
  0 siblings, 2 replies; 6+ messages in thread
From: Maxime Coquelin @ 2024-02-26 11:03 UTC (permalink / raw)
  To: Chautru, Nicolas, dev, Vargas, Hernan, Marchand, David, thomas, roretzla

Hello,

On 2/22/24 19:05, Chautru, Nicolas wrote:
> Hi Maxime,
> 
> Why would we change this here and now? Is the intent not to use new suggested semantics for new patches only?

The pull request was rejected because of the use of such barrier, which 
is reported by checkpatch.

### [PATCH] baseband/fpga_5gnr_fec: add AGX100 support
Warning in drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c:
Using rte_smp_[r/w]mb

> Are all DPDK drivers being changed?

My understanding is that for now, only new occurrences are prohibited,
can you confirm Tyler?

If so we could only change for now the patch adding ACX100.
But... I preferred doing the changes for all bbdev drivers for
consistency.

> I am unsure we would want to change these drivers, this is kind of risk introduced by code churn that gets ecosystem unwilling to move to latest version.

I think it is better to change now that we are far from the next LTS.

> These memory barriers issues are awful to troubleshoot or properly validate, so personally quite reluctant to change.

If I disassemble fpga_dequeue_enc() with and without the patch, I cannot
spot a difference.

Thomas, are you waiting for this series to be applied to take the pull
request that was initially for -rc1?

Thanks,
Maxime

> Thanks
> Nic
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, February 22, 2024 8:21 AM
>> To: dev@dpdk.org; Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas,
>> Hernan <hernan.vargas@intel.com>; Marchand, David
>> <david.marchand@redhat.com>; thomas@monjalon.net;
>> roretzla@linux.microsoft.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
>>
>> rte_smp_rmb() is deprecated, use the new API instead as suggested in
>> rte_atomic header.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>> index efc1d3a772..314c87350e 100644
>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>> @@ -2661,7 +2661,7 @@ vc_5gnr_dequeue_ldpc_enc_one_op_cb(struct
>> fpga_5gnr_queue *q, struct rte_bbdev_e
>>   		return -1;
>>
>>   	/* make sure the response is read atomically */
>> -	rte_smp_rmb();
>> +	rte_atomic_thread_fence(rte_memory_order_acquire);
>>
>>   	rte_bbdev_log_debug("DMA response desc %p", desc);
>>
>> @@ -2690,7 +2690,7 @@ agx100_dequeue_ldpc_enc_one_op_cb(struct
>> fpga_5gnr_queue *q, struct rte_bbdev_en
>>   		return -1;
>>
>>   	/* make sure the response is read atomically. */
>> -	rte_smp_rmb();
>> +	rte_atomic_thread_fence(rte_memory_order_acquire);
>>
>>   	rte_bbdev_log_debug("DMA response desc %p", desc);
>>
>> @@ -2722,7 +2722,7 @@ vc_5gnr_dequeue_ldpc_dec_one_op_cb(struct
>> fpga_5gnr_queue *q, struct rte_bbdev_d
>>   		return -1;
>>
>>   	/* make sure the response is read atomically */
>> -	rte_smp_rmb();
>> +	rte_atomic_thread_fence(rte_memory_order_acquire);
>>
>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>   	vc_5gnr_print_dma_dec_desc_debug_info(desc);
>> @@ -2768,7 +2768,7 @@ agx100_dequeue_ldpc_dec_one_op_cb(struct
>> fpga_5gnr_queue *q, struct rte_bbdev_de
>>   		return -1;
>>
>>   	/* make sure the response is read atomically. */
>> -	rte_smp_rmb();
>> +	rte_atomic_thread_fence(rte_memory_order_acquire);
>>
>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>   	agx100_print_dma_dec_desc_debug_info(desc);
>> --
>> 2.43.0
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
  2024-02-26 11:03   ` Maxime Coquelin
@ 2024-02-27 16:56     ` Maxime Coquelin
  2024-08-12  9:58     ` Maxime Coquelin
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2024-02-27 16:56 UTC (permalink / raw)
  To: roretzla; +Cc: Chautru, Nicolas, dev, Vargas, Hernan, thomas, Marchand, David

Hi Tyler,

On 2/26/24 12:03, Maxime Coquelin wrote:
> Hello,
> 
> On 2/22/24 19:05, Chautru, Nicolas wrote:
>> Hi Maxime,
>>
>> Why would we change this here and now? Is the intent not to use new 
>> suggested semantics for new patches only?
> 
> The pull request was rejected because of the use of such barrier, which 
> is reported by checkpatch.
> 
> ### [PATCH] baseband/fpga_5gnr_fec: add AGX100 support
> Warning in drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c:
> Using rte_smp_[r/w]mb
> 
>> Are all DPDK drivers being changed?
> 
> My understanding is that for now, only new occurrences are prohibited,
> can you confirm Tyler?

Could you please clarify whether conversion for existing rte_smp_rmb()
calls to rte_atomic_thread_fence() in drivers is planned, or only new
occurrences are prohibited?

Thanks in advances,
Maxime

> If so we could only change for now the patch adding ACX100.
> But... I preferred doing the changes for all bbdev drivers for
> consistency.
> 
>> I am unsure we would want to change these drivers, this is kind of 
>> risk introduced by code churn that gets ecosystem unwilling to move to 
>> latest version.
> 
> I think it is better to change now that we are far from the next LTS.
> 
>> These memory barriers issues are awful to troubleshoot or properly 
>> validate, so personally quite reluctant to change.
> 
> If I disassemble fpga_dequeue_enc() with and without the patch, I cannot
> spot a difference.
> 
> Thomas, are you waiting for this series to be applied to take the pull
> request that was initially for -rc1?
> 
> Thanks,
> Maxime
> 
>> Thanks
>> Nic
>>
>>> -----Original Message-----
>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Sent: Thursday, February 22, 2024 8:21 AM
>>> To: dev@dpdk.org; Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas,
>>> Hernan <hernan.vargas@intel.com>; Marchand, David
>>> <david.marchand@redhat.com>; thomas@monjalon.net;
>>> roretzla@linux.microsoft.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
>>>
>>> rte_smp_rmb() is deprecated, use the new API instead as suggested in
>>> rte_atomic header.
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> ---
>>>   drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> index efc1d3a772..314c87350e 100644
>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> @@ -2661,7 +2661,7 @@ vc_5gnr_dequeue_ldpc_enc_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_e
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>       rte_bbdev_log_debug("DMA response desc %p", desc);
>>>
>>> @@ -2690,7 +2690,7 @@ agx100_dequeue_ldpc_enc_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_en
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically. */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>       rte_bbdev_log_debug("DMA response desc %p", desc);
>>>
>>> @@ -2722,7 +2722,7 @@ vc_5gnr_dequeue_ldpc_dec_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_d
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>>       vc_5gnr_print_dma_dec_desc_debug_info(desc);
>>> @@ -2768,7 +2768,7 @@ agx100_dequeue_ldpc_dec_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_de
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically. */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>>       agx100_print_dma_dec_desc_debug_info(desc);
>>> -- 
>>> 2.43.0
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
  2024-02-26 11:03   ` Maxime Coquelin
  2024-02-27 16:56     ` Maxime Coquelin
@ 2024-08-12  9:58     ` Maxime Coquelin
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2024-08-12  9:58 UTC (permalink / raw)
  To: Chautru, Nicolas, dev, Vargas, Hernan, Marchand, David, thomas, roretzla

Hello Nicolas,

What do we do with this series?
Do we take it for v24.11?

Thanks,
Maxime

On 2/26/24 12:03, Maxime Coquelin wrote:
> Hello,
> 
> On 2/22/24 19:05, Chautru, Nicolas wrote:
>> Hi Maxime,
>>
>> Why would we change this here and now? Is the intent not to use new 
>> suggested semantics for new patches only?
> 
> The pull request was rejected because of the use of such barrier, which 
> is reported by checkpatch.
> 
> ### [PATCH] baseband/fpga_5gnr_fec: add AGX100 support
> Warning in drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c:
> Using rte_smp_[r/w]mb
> 
>> Are all DPDK drivers being changed?
> 
> My understanding is that for now, only new occurrences are prohibited,
> can you confirm Tyler?
> 
> If so we could only change for now the patch adding ACX100.
> But... I preferred doing the changes for all bbdev drivers for
> consistency.
> 
>> I am unsure we would want to change these drivers, this is kind of 
>> risk introduced by code churn that gets ecosystem unwilling to move to 
>> latest version.
> 
> I think it is better to change now that we are far from the next LTS.
> 
>> These memory barriers issues are awful to troubleshoot or properly 
>> validate, so personally quite reluctant to change.
> 
> If I disassemble fpga_dequeue_enc() with and without the patch, I cannot
> spot a difference.
> 
> Thomas, are you waiting for this series to be applied to take the pull
> request that was initially for -rc1?
> 
> Thanks,
> Maxime
> 
>> Thanks
>> Nic
>>
>>> -----Original Message-----
>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Sent: Thursday, February 22, 2024 8:21 AM
>>> To: dev@dpdk.org; Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas,
>>> Hernan <hernan.vargas@intel.com>; Marchand, David
>>> <david.marchand@redhat.com>; thomas@monjalon.net;
>>> roretzla@linux.microsoft.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API
>>>
>>> rte_smp_rmb() is deprecated, use the new API instead as suggested in
>>> rte_atomic header.
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> ---
>>>   drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> index efc1d3a772..314c87350e 100644
>>> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
>>> @@ -2661,7 +2661,7 @@ vc_5gnr_dequeue_ldpc_enc_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_e
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>       rte_bbdev_log_debug("DMA response desc %p", desc);
>>>
>>> @@ -2690,7 +2690,7 @@ agx100_dequeue_ldpc_enc_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_en
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically. */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>       rte_bbdev_log_debug("DMA response desc %p", desc);
>>>
>>> @@ -2722,7 +2722,7 @@ vc_5gnr_dequeue_ldpc_dec_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_d
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>>       vc_5gnr_print_dma_dec_desc_debug_info(desc);
>>> @@ -2768,7 +2768,7 @@ agx100_dequeue_ldpc_dec_one_op_cb(struct
>>> fpga_5gnr_queue *q, struct rte_bbdev_de
>>>           return -1;
>>>
>>>       /* make sure the response is read atomically. */
>>> -    rte_smp_rmb();
>>> +    rte_atomic_thread_fence(rte_memory_order_acquire);
>>>
>>>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>>>       agx100_print_dma_dec_desc_debug_info(desc);
>>> -- 
>>> 2.43.0
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-08-12  9:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 16:21 [PATCH 1/2] baseband/fpga_5gnr_fec: use new barrier API Maxime Coquelin
2024-02-22 16:21 ` [PATCH 2/2] baseband/fpga_lte_fec: " Maxime Coquelin
2024-02-22 18:05 ` [PATCH 1/2] baseband/fpga_5gnr_fec: " Chautru, Nicolas
2024-02-26 11:03   ` Maxime Coquelin
2024-02-27 16:56     ` Maxime Coquelin
2024-08-12  9:58     ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).