rcu: fix build failure with debug dp log level

PATCH Superseded Anoob Joseph 6 participants 10 messages Aug 29, 2022 – Oct 6, 2022

Share & tools

Honnappa Nagarahalli

2022-08-29 16:56 UTC #1 lore raw

Quoted 13 lines

-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Monday, August 29, 2022 11:52 AM
To: Honnappa Nagarahalli <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] rcu: fix build failure with debug dp log level
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.

Quoted 6 lines

../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]

Agree on the fix.
@[email protected] Does the process that removed this header file inclusion needs fixing? If yes, should that fix be included in this patch?

Quoted 23 lines

Signed-off-by Anoob Joseph <[email protected]>
---
 lib/rcu/rte_rcu_qsbr.h | 4 ++++
 1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h +4 -1
@@ -37,6 +37,10 @@ extern "C" {
 #include <rte_atomic.h>
 #include <rte_ring.h>
 
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
+#include <inttypes.h>
+#endif
+
 extern int rte_rcu_log_type;
 
 #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
--
2.25.1

Morrissey, Sean

2022-08-30 08:38 UTC #2 lore raw

Quoted 15 lines · Honnappa Nagarahalli

Quoted 15 lines
-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Monday, August 29, 2022 11:52 AM
To: Honnappa Nagarahalli <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] rcu: fix build failure with debug dp log level
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~

Quoted 5 lines

Quoted 2 lines
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Agree on the fix.
@[email protected] Does the process that removed this header file inclusion needs fixing? If yes, should that fix be included in this patch?

@[email protected] Yes, I believe the tool will need an update, however, I believe it should be a separate patch for that.

Quoted 22 lines

Quoted 22 lines
Signed-off-by Anoob Joseph <[email protected]>
---
  lib/rcu/rte_rcu_qsbr.h | 4 ++++
  1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h +4 -1
@@ -37,6 +37,10 @@ extern "C" {
  #include <rte_atomic.h>
  #include <rte_ring.h>
 
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
+#include <inttypes.h>
+#endif
+
  extern int rte_rcu_log_type;
 
  #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
--
2.25.1

Honnappa Nagarahalli

2022-08-30 13:17 UTC #3 lore raw

<snip>

Quoted 11 lines

Quoted 11 lines
Quoted 11 lines
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]

Quoted 7 lines

Quoted 2 lines
Agree on the fix.
@[email protected] Does the process that removed this header file
inclusion needs fixing?
Quoted 1 line
If yes, should that fix be included in this patch?
@[email protected] Yes, I believe the tool will need an update, however, I believe it should be a separate patch for that.

Ok, as long as it is addressed, it should be fine.

Quoted 21 lines

Quoted 21 lines
Quoted 21 lines
Signed-off-by Anoob Joseph <[email protected]>
---
  lib/rcu/rte_rcu_qsbr.h | 4 ++++
  1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h +2 -1
@@ -37,6 +37,10 @@ extern "C" {
  #include <rte_atomic.h>
  #include <rte_ring.h>
 
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG #include <inttypes.h> #endif
+
  extern int rte_rcu_log_type;
 
  #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
--
2.25.1

Honnappa Nagarahalli

Reviewed-by

2022-09-28 22:04 UTC #4 lore raw

<snip>

Quoted 15 lines

Quoted 15 lines
Quoted 15 lines
Quoted 6 lines
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
Quoted 6 lines
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Agree on the fix.
@[email protected] Does the process that removed this header file

Quoted 9 lines

Quoted 5 lines
inclusion needs fixing?
Quoted 1 line
If yes, should that fix be included in this patch?
@[email protected] Yes, I believe the tool will need an update, however, I believe it should be a separate patch for that.
Ok, as long as it is addressed, it should be fine.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Signed-off-by Anoob Joseph <[email protected]>

Reviewed-by Honnappa Nagarahalli <[email protected]>

Quoted 20 lines

Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
---
  lib/rcu/rte_rcu_qsbr.h | 4 ++++
  1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h +0 -0
@@ -37,6 +37,10 @@ extern "C" {
  #include <rte_atomic.h>
  #include <rte_ring.h>
 
Quoted 1 line
Quoted 1 line
lib/rcu/rte_rcu_qsbr.h extern "C" { · line 40
Quoted 1 line
lib/rcu/rte_rcu_qsbr.h +1 -0
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG #include <inttypes.h>
#endif
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
lib/rcu/rte_rcu_qsbr.h +1 -1
+
  extern int rte_rcu_log_type;
 
  #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
--
2.25.1

Stephen Hemminger

2022-09-28 22:21 UTC #5 lore raw

In reply to Anoob Joseph

Quoted 21 lines · On Mon, 29 Aug 2022 22:21:51 +0530 Anoob Joseph <[email protected]> wrote:

Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Signed-off-by Anoob Joseph <[email protected]>
---
 lib/rcu/rte_rcu_qsbr.h | 4 ++++
 1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h +0 -0
index d81bf5e8db..b0f1720ca1 100644

lib/rcu/rte_rcu_qsbr.h extern "C" { · line 44

Quoted 8 lines

lib/rcu/rte_rcu_qsbr.h +4 -0

@@ -37,6 +37,10 @@ extern "C" {
 #include <rte_atomic.h>
 #include <rte_ring.h>
 
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
+#include <inttypes.h>
+#endif
+
 extern int rt

This is not the best way to fix this.
Just always include the header file.
Having it conditional can lead to more problems

Anoob Joseph

author

2022-09-29 03:25 UTC #6 lore raw

Hi Stephen,

Please see inline.

Thanks,
Anoob

Quoted 39 lines

-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Thursday, September 29, 2022 3:51 AM
To: Anoob Joseph <[email protected]>
Cc: Honnappa Nagarahalli <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH] rcu: fix build failure with debug dp log level
External Email
----------------------------------------------------------------------
Quoted 24 lines · On Mon, 29 Aug 2022 22:21:51 +0530 Anoob Joseph <[email protected]> wrote:
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Signed-off-by Anoob Joseph <[email protected]>
---
 lib/rcu/rte_rcu_qsbr.h | 4 ++++
 1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h +0 -0
@@ -37,6 +37,10 @@ extern "C" {
 #include <rte_atomic.h>
 #include <rte_ring.h>
 

Quoted 8 lines

lib/rcu/rte_rcu_qsbr.h extern "C" { · line 40
Quoted 1 line
lib/rcu/rte_rcu_qsbr.h +1 -0
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG #include <inttypes.h>
#endif
lib/rcu/rte_rcu_qsbr.h extern "C" { · line 42
Quoted 2 lines
lib/rcu/rte_rcu_qsbr.h +1 -0
+
 extern int rt
This is not the best way to fix this.
Just always include the header file.
Having it conditional can lead to more problems

[Anoob] The header include is only required when RTE_LOG_DP_LEVEL is lower than RTE_LOG_DEBUG. I'm not sure how the tool runs, but it may still flag this include as an unnecessary include. But I can make the change if the tool can ignore this case.

@Sean, do you agree with the suggestion?

Bruce Richardson

2022-09-29 08:28 UTC #7 lore raw

Quoted 48 lines · Anoob Joseph

Hi Stephen,
Please see inline.
Thanks,
Anoob
Quoted 41 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Thursday, September 29, 2022 3:51 AM
To: Anoob Joseph <[email protected]>
Cc: Honnappa Nagarahalli <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH] rcu: fix build failure with debug dp log level
External Email
----------------------------------------------------------------------
Quoted 21 lines · On Mon, 29 Aug 2022 22:21:51 +0530 Anoob Joseph <[email protected]> wrote:
Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header when RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Signed-off-by Anoob Joseph <[email protected]>
---
 lib/rcu/rte_rcu_qsbr.h | 4 ++++
 1 file changed, 4 insertions(+)
lib/rcu/rte_rcu_qsbr.h index renamed from lib/rcu/rte_rcu_qsbr.h +0 -0
d81bf5e8db..b0f1720ca1 100644
lib/rcu/rte_rcu_qsbr.h extern "C" { · line 40
Quoted 4 lines
lib/rcu/rte_rcu_qsbr.h +1 -0
@@ -37,6 +37,10 @@ extern "C" {
 #include <rte_atomic.h>
 #include <rte_ring.h>
 
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG #include <inttypes.h>
#endif

Quoted 9 lines

Quoted 6 lines
lib/rcu/rte_rcu_qsbr.h extern "C" { · line 42
Quoted 2 lines
lib/rcu/rte_rcu_qsbr.h +1 -0
+
 extern int rt
This is not the best way to fix this.
Just always include the header file.
Having it conditional can lead to more problems
[Anoob] The header include is only required when RTE_LOG_DP_LEVEL is lower than RTE_LOG_DEBUG. I'm not sure how the tool runs, but it may still flag this include as an unnecessary include. But I can make the change if the tool can ignore this case.

I think the number of build configurations needing testing has made the automatic removal of header includes too problematic to use further. Therefore, I think either way - with or without the #if - is probably fine.

/Bruce

Anoob Joseph

author

2022-09-29 10:27 UTC #8 lore raw

PATCH v2 1/1 rcu: fix build failure with debug dp log level

In reply to Anoob Joseph

Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header.

../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~

Fixes 30a1de105a5f ("lib: remove unneeded header includes")

Cc [email protected]

Signed-off-by Anoob Joseph <[email protected]>

---
 lib/rcu/rte_rcu_qsbr.h | 1 +
 1 file changed, 1 insertion(+)

lib/rcu/rte_rcu_qsbr.h +1 -0

index d81bf5e8db..36571c15df 100644
@@ -29,6 +29,7 @@
 extern "C" {
 #endif
 
+#include <inttypes.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdint.h>

--
2.25.1

Thomas Monjalon

Applied

2022-10-06 10:40 UTC #9 lore raw

29/09/2022 12:27, Anoob Joseph:

Quoted 11 lines

Build fails if RTE_LOG_DP_LEVEL is set to RTE_LOG_DEBUG. Fix the same by including the required header.
../lib/rcu/rte_rcu_qsbr.h:678:40: error: expected ‘)’ before ‘PRIu64’
678 | "%s: status: least acked token = %" PRIu64,
| ^~~~~~
Fixes 30a1de105a5f ("lib: remove unneeded header includes")
Cc [email protected]
Signed-off-by Anoob Joseph <[email protected]>

Applied, thanks.

Would be good to avoid the tool doing the same mistake again.