Hi,

I like your patch.
You follow PRINT_RL, good thinking.

I want to recommend:

  1.  You need to commit the changes locally. You need to sign the patches.
And create patches for send with “git format-patch <your base version> 
--cover-letter”.
You need to fill the cover letter “patch” manually.
The cover letter “patch” is a general message and a summery of the patch series.
A single patch need a cover letter “patch” too.
On a second version or higher use the version ‘-v’ flag, i.e.  “git 
format-patch <your base version> -v2 --cover-letter”.
You can use the ‘-o’ output flag to place the patches in a different folder.
  2.  The patches, including the cover letter “patch” need to pass the Linux 
kernel scripts/checkpatch.pl.
  3.  You need to send the patch series with the cover letter using “git 
send-email”.

Regarding the patch itself.
Why not using a global for level, so we can skip the function call?
I would use an inline function in the print header for better reading.
Simply move print_get_level()to header and make it inline and make print_level 
a global.

The rate_limited function call a syscall function, clock_gettime().
So making the function inline would not help much.

Anyhow, the pl_xxx are rarely used, while pr_xxx is everywhere.
linuxptp (master)$ grep '\<pr_' *.[ch] | wc
    543    3385   31408
linuxptp (master)$ grep '\<pl_' *.[ch] | wc
     13      86     813

Erez

P.S.
We usually reply with inline.
My company M$ outlook fails ...


From: Kosta Demirev <kosta_demi...@trimble.com>
Sent: Friday, 17 December 2021 15:27
To: linuxptp-devel@lists.sourceforge.net
Subject: [Linuxptp-devel] linuxptp performance hit

My apologies, I didn't submit a clean patch.
Probably this submission will clear it.
Thanks.

diff --git a/print.h b/print.h
index 1723d8a..7e765da 100644
--- a/print.h
+++ b/print.h
@@ -38,14 +38,32 @@ void print_set_syslog(int value);
 void print_set_level(int level);
 void print_set_verbose(int value);

-#define pr_emerg(x...)   print(LOG_EMERG, x)
-#define pr_alert(x...)   print(LOG_ALERT, x)
-#define pr_crit(x...)    print(LOG_CRIT, x)
-#define pr_err(x...)     print(LOG_ERR, x)
-#define pr_warning(x...) print(LOG_WARNING, x)
-#define pr_notice(x...)  print(LOG_NOTICE, x)
-#define pr_info(x...)    print(LOG_INFO, x)
-#define pr_debug(x...)   print(LOG_DEBUG, x)
+  /*
+   * Better check print log level before execution of print itself.
+   * Otherwise all arguments are evaluated and slow down the system.
+   * e.g.   in 'unicast_service.c' unicast_service_clients()
+   *                            pid2str() is the killer
+   *           pr_debug("%s wants 0x%x", pid2str(&client->portIdentity),
+   *                    client->message_types);
+   */
+
+  extern
+  int print_get_level( void);
+
+  #define PRINT_CL(l, x...) /* PRINT Check Level */    \
+       do {                                            \
+               if ( print_get_level() >= l)            \
+                       print(l, x);                    \
+       } while (0)
+
+  #define pr_emerg(x...)   PRINT_CL(LOG_EMERG, x)
+  #define pr_alert(x...)   PRINT_CL(LOG_ALERT, x)
+  #define pr_crit(x...)    PRINT_CL(LOG_CRIT, x)
+  #define pr_err(x...)     PRINT_CL(LOG_ERR, x)
+  #define pr_warning(x...) PRINT_CL(LOG_WARNING, x)
+  #define pr_notice(x...)  PRINT_CL(LOG_NOTICE, x)
+  #define pr_info(x...)    PRINT_CL(LOG_INFO, x)
+  #define pr_debug(x...)   PRINT_CL(LOG_DEBUG, x)

 #define PRINT_RL(l, i, x...) \
        do { \

diff --git a/print.c b/print.c
index c428895..bf70aca 100644
--- a/print.c
+++ b/print.c
@@ -50,6 +50,11 @@ void print_set_level(int level)
        print_level = level;
 }

+int print_get_level( void)
+{
+       return print_level;
+}
+
 void print_set_verbose(int value)
 {
        verbose = value ? 1 : 0;


Kosta
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to