Hi Erez,
I am almost giving up on making my first patch.
After following all the recommendations, I failed on 'git send-email'.
I am attaching needed files for the patch( use it as you like).
I spend my day for this patch, instead to figure out why 'ptp4l' in unicast
mode, IPv6(UDPv6) does not
send all DResp messages to Dreq when in high load( much more interesting
than 'git send-email').
Thank You guys
Kosta
Here is the failure:
kosta@T460:/opt/Trimble.opt/test.git/ptp4l-3.1/linuxptp.git/linuxptp-code.sf$
git send-email ../patches/
../patches/0000-cover-letter.patch
../patches/0001-Check-print_log-before-arguments-are-evaluated-not.patch
To whom should the emails be sent (if anyone)?
linuxptp-devel@lists.sourceforge.net
Message-ID to be used as In-Reply-To for the first email (if any)?
(mbox) Adding cc: Kosta Demirev <kosta_demi...@trimble.com> from line
'From: Kosta Demirev <kosta_demi...@trimble.com>'
From: Kosta Demirev <kosta_demi...@trimble.com>
To: linuxptp-devel@lists.sourceforge.net
Cc: Kosta Demirev <kosta_demi...@trimble.com>
Subject: [PATCH 0/1] Check 'print_log' before arguments are evaluated, not
after. Performance wise patch. <kdemi...@gmail.com>
Date: Fri, 17 Dec 2021 14:47:17 -0800
Message-Id: <20211217224718.105329-1-kosta_demi...@trimble.com>
X-Mailer: git-send-email 2.25.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.
For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
Unable to initialize SMTP properly. Check config and use --smtp-debug.
VALUES: server=localhost encryption= hello=T460.home port=25 at
/usr/lib/git-core/git-send-email line 1558.
On Fri, Dec 17, 2021 at 10:34 AM Geva, Erez <erez.geva....@siemens.com>
wrote:
> 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
>
From 824a2953a1119e1efb28221668ac9615572d6309 Mon Sep 17 00:00:00 2001
From: Kosta Demirev <kosta_demi...@trimble.com>
Date: Fri, 17 Dec 2021 12:59:39 -0800
Subject: [PATCH 0/1] Check 'print_log' before arguments are evaluated, not
after. Performance wise patch. <kdemi...@gmail.com>
Kosta Demirev (1):
Check 'print_log' before arguments are evaluated, not after.
Performance wise patch. <kdemi...@gmail.com>
print.c | 2 +-
print.h | 40 ++++++++++++++++++++++++++++++++--------
2 files changed, 33 insertions(+), 9 deletions(-)
--
2.25.1
From 824a2953a1119e1efb28221668ac9615572d6309 Mon Sep 17 00:00:00 2001
From: Kosta Demirev <kosta_demi...@trimble.com>
Date: Fri, 17 Dec 2021 12:57:31 -0800
Subject: [PATCH 1/1]
Check 'print_log' before arguments are evaluated, not
after. Performance wise patch. <kdemi...@gmail.com>
Signed-off-by: Kosta Demirev <kosta_demi...@trimble.com>
---
print.c | 2 +-
print.h | 40 ++++++++++++++++++++++++++++++++--------
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/print.c b/print.c
index c428895..02bdcd0 100644
--- a/print.c
+++ b/print.c
@@ -25,7 +25,7 @@
#include "print.h"
static int verbose = 0;
-static int print_level = LOG_INFO;
+int print_level = LOG_INFO;
static int use_syslog = 1;
static const char *progname;
static const char *message_tag;
diff --git a/print.h b/print.h
index 1723d8a..334809f 100644
--- a/print.h
+++ b/print.h
@@ -38,14 +38,38 @@ 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_level;
+
+inline
+int print_get_level(void)
+{
+ return print_level;
+}
+
+#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 { \
--
2.25.1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
