From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmmFS-0005mW-3E for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:23:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZmmFO-0006sU-UP for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:23:26 -0400 Received: from mail-lb0-f172.google.com ([209.85.217.172]:33641) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmmFO-0006sL-JI for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:23:22 -0400 Received: by lbbpp2 with SMTP id pp2so48900302lbb.0 for ; Thu, 15 Oct 2015 10:23:21 -0700 (PDT) References: <1444894224-9542-1-git-send-email-den@openvz.org> <1444894224-9542-2-git-send-email-den@openvz.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1444894224-9542-2-git-send-email-den@openvz.org> Date: Thu, 15 Oct 2015 18:23:19 +0100 Message-ID: <8737xc6o6w.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/3] log: improve performance of qemu_log and qemu_log_mask if disabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: qemu-devel@nongnu.org, Peter Maydell , Luiz Capitulino , Markus Armbruster , Pavel Butsykin Denis V. Lunev writes: > The patch is intended to avoid to perform any operation including > calculation of log function arguments when the log is not enabled due to > various reasons. > > Functions qemu_log and qemu_log_mask are replaced with variadic macros. > Unfortunately the code is not C99 compatible and we can not use > portable __VA_ARGS__ way. There are a lot of warnings in the other > places with --std=c99. Thus the only way to achive the result is to use > args.. GCC extension. > > Format checking performed by compiler will not suffer by this patch. It > will be done inside in fprintf arguments checking. > > Signed-off-by: Denis V. Lunev > Signed-off-by: Pavel Butsykin > CC: Markus Armbruster > CC: Luiz Capitulino > CC: Eric Blake > CC: Peter Maydell > --- > include/qemu/log.h | 17 ++++++++++++++--- > qemu-log.c | 21 --------------------- > 2 files changed, 14 insertions(+), 24 deletions(-) > > diff --git a/include/qemu/log.h b/include/qemu/log.h > index f880e66..57b8c96 100644 > --- a/include/qemu/log.h > +++ b/include/qemu/log.h > @@ -53,7 +53,13 @@ static inline bool qemu_loglevel_mask(int mask) > > /* main logging function > */ > -void GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...); > +#define qemu_log(args...) \ > + do { \ > + if (!qemu_log_enabled()) { \ > + break; \ > + } \ > + fprintf(qemu_logfile, args); \ > + } while (0) > I've had one of Peter's patches in my logging improvements queue for a while although it uses a slightly different form which I prefer: -/* log only if a bit is set on the current loglevel mask +/* log only if a bit is set on the current loglevel mask: + * @mask: bit to check in the mask + * @fmt: printf-style format string + * @args: optional arguments for format string */ -void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...); - +#define qemu_log_mask(MASK, FMT, ...) \ + do { \ + if (unlikely(qemu_loglevel_mask(MASK))) { \ + qemu_log(FMT, ## __VA_ARGS__); \ + } \ See the message: qemu-log: Avoid function call for disabled qemu_log_mask logging > /* vfprintf-like logging function > */ > @@ -67,8 +73,13 @@ qemu_log_vprintf(const char *fmt, va_list va) > > /* log only if a bit is set on the current loglevel mask > */ > -void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...); > - > +#define qemu_log_mask(mask, args...) \ > + do { \ > + if (!qemu_loglevel_mask(mask)) { \ > + break; \ > + } \ > + qemu_log(args); \ > + } while (0) > > /* Special cases: */ > > diff --git a/qemu-log.c b/qemu-log.c > index 13f3813..e6d2b3f 100644 > --- a/qemu-log.c > +++ b/qemu-log.c > @@ -25,27 +25,6 @@ FILE *qemu_logfile; > int qemu_loglevel; > static int log_append = 0; > > -void qemu_log(const char *fmt, ...) > -{ > - va_list ap; > - > - va_start(ap, fmt); > - if (qemu_logfile) { > - vfprintf(qemu_logfile, fmt, ap); > - } > - va_end(ap); > -} > - > -void qemu_log_mask(int mask, const char *fmt, ...) > -{ > - va_list ap; > - > - va_start(ap, fmt); > - if ((qemu_loglevel & mask) && qemu_logfile) { > - vfprintf(qemu_logfile, fmt, ap); > - } > - va_end(ap); > -} > > /* enable or disable low levels log */ > void do_qemu_set_log(int log_flags, bool use_own_buffers) -- Alex Bennée