| Message ID | 1457954501-26528-6-git-send-email-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14/03/2016 12:21, Denis V. Lunev wrote: > 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. > > Format checking performed by compiler will not suffer by this patch. It > will be done inside in fprintf arguments checking. Have you encountered a place that was calling them so hard that it caused performance problem? If so, the logging should probably be replaced by a tracepoint. Paolo > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Paolo Bonzini <pbonzini@redhat.com> > --- > include/qemu/log.h | 15 ++++++++++++--- > util/log.c | 21 --------------------- > 2 files changed, 12 insertions(+), 24 deletions(-) > > diff --git a/include/qemu/log.h b/include/qemu/log.h > index 2c2c220..a05c7dc 100644 > --- a/include/qemu/log.h > +++ b/include/qemu/log.h > @@ -54,7 +54,12 @@ 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(...) \ > + do { \ > + if (qemu_log_enabled()) { \ > + fprintf(qemu_logfile, __VA_ARGS__); \ > + } \ > + } while (0) > > /* vfprintf-like logging function > */ > @@ -68,8 +73,12 @@ 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, ...) \ > + do { \ > + if ((qemu_loglevel & (mask)) && qemu_log_enabled()) { \ > + fprintf(qemu_logfile, __VA_ARGS__); \ > + } \ > + } while (0) > > /* Maintenance: */ > > diff --git a/util/log.c b/util/log.c > index df672cc..65d46e2 100644 > --- a/util/log.c > +++ b/util/log.c > @@ -27,27 +27,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) >
On 03/14/2016 05:30 PM, Paolo Bonzini wrote: > > On 14/03/2016 12:21, Denis V. Lunev wrote: >> 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. >> >> Format checking performed by compiler will not suffer by this patch. It >> will be done inside in fprintf arguments checking. > Have you encountered a place that was calling them so hard that it > caused performance problem? If so, the logging should probably be > replaced by a tracepoint. > > Paolo > inline functions can have side-effects, i.e. the arguments of them can be evaluated depending on the compiler. with this approach we are sure that this will not happen, ever. Den
diff --git a/include/qemu/log.h b/include/qemu/log.h index 2c2c220..a05c7dc 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -54,7 +54,12 @@ 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(...) \ + do { \ + if (qemu_log_enabled()) { \ + fprintf(qemu_logfile, __VA_ARGS__); \ + } \ + } while (0) /* vfprintf-like logging function */ @@ -68,8 +73,12 @@ 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, ...) \ + do { \ + if ((qemu_loglevel & (mask)) && qemu_log_enabled()) { \ + fprintf(qemu_logfile, __VA_ARGS__); \ + } \ + } while (0) /* Maintenance: */ diff --git a/util/log.c b/util/log.c index df672cc..65d46e2 100644 --- a/util/log.c +++ b/util/log.c @@ -27,27 +27,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)
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. 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 <den@openvz.org> CC: Stefan Hajnoczi <stefanha@redhat.com> CC: Paolo Bonzini <pbonzini@redhat.com> --- include/qemu/log.h | 15 ++++++++++++--- util/log.c | 21 --------------------- 2 files changed, 12 insertions(+), 24 deletions(-)