[ClusterLabs Developers] libqb: Re: 633f262 logging: Remove linker 'magic' and just use statics for logging callsites (#322)

Lars Ellenberg lars.ellenberg at linbit.com
Wed Jan 16 10:20:51 EST 2019


On Wed, Jan 16, 2019 at 03:44:22PM +0100, Lars Ellenberg wrote:
> Back then when this "dynamic" logging was introduced,
> I thought the whole point was that "quiet" callsites
> are "cheap".
> 
> So I think you want to
> qb_log_callsite_get() only *once* per callsite,
> assign that to a static pointer (as you say in the commit message).
> And do the actual qb_log_real_() function call
> only conditionally based on if (cs->targets). 
> 
> That way, a disabled trace logging boils down to a
> if (cs && cs->targets)
>     ;
> Much cheaper than what you have now.
> 
> Or was always calling into both qb_log_callsite_get() and qb_log_real_()
> intentional for some obscure (to me) reason,
> even for disabled call sites?
> 
> Below I also added a test for (cs->tags & QB_LOG_TAG_LIBQB_MSG),
> in case someone used qb_util_set_log_function().
> If that special tag flag could be folded into cs->targets (e.g. bit 0),
> I'd like it even better.
> 
> Cheers,
>     Lars
> 
> 
> diff --git a/include/qb/qblog.h b/include/qb/qblog.h
> index 1943b94..f63f4ad 100644

Oops, that patch was against *before* the 633f262 commit :-)
and that's why I did not notice the clash in macro argument "tags"
and struct member "tags" when compile testing...
I forgot I jumped between checkouts :-(

anyways, the (now even make check testet)
patch for *after* 633f262 commit:

diff --git a/include/qb/qblog.h b/include/qb/qblog.h
index 31981b8..ae1d25c 100644
--- a/include/qb/qblog.h
+++ b/include/qb/qblog.h
@@ -340,11 +340,17 @@ void qb_log_from_external_source_va(const char *function,
  * @param fmt usual printf style format specifiers
  * @param args usual printf style args
  */
-#define qb_logt(priority, tags, fmt, args...) do {	\
-	struct qb_log_callsite* descriptor_pt =		\
-	qb_log_callsite_get(__func__, __FILE__, fmt,	\
-			    priority, __LINE__, tags);	\
-	qb_log_real_(descriptor_pt, ##args);		\
+#define qb_logt(priority, tags_, fmt, args...) do {		\
+	static struct qb_log_callsite* descriptor_pt;		\
+	if (!descriptor_pt) {					\
+		descriptor_pt =					\
+		qb_log_callsite_get(__func__, __FILE__, fmt,	\
+				    priority, __LINE__, tags_);	\
+	}							\
+	if (descriptor_pt && (descriptor_pt->targets ||		\
+	    qb_bit_set(descriptor_pt->tags,			\
+		    QB_LOG_TAG_LIBQB_MSG_BIT)))			\
+		qb_log_real_(descriptor_pt, ##args);		\
     } while(0)
 



More information about the Developers mailing list