From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmmWN-0003Cz-FW for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:40:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZmmWI-00052k-9q for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:40:55 -0400 Received: from mx2.parallels.com ([199.115.105.18]:45218) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmmWI-00051l-47 for qemu-devel@nongnu.org; Thu, 15 Oct 2015 13:40:50 -0400 References: <1444894224-9542-1-git-send-email-den@openvz.org> <1444894224-9542-2-git-send-email-den@openvz.org> <8737xc6o6w.fsf@linaro.org> From: "Denis V. Lunev" Message-ID: <561FE515.7030402@openvz.org> Date: Thu, 15 Oct 2015 20:40:37 +0300 MIME-Version: 1.0 In-Reply-To: <8737xc6o6w.fsf@linaro.org> Content-Type: text/plain; charset="utf-8"; format=flowed 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: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: qemu-devel@nongnu.org, Peter Maydell , Luiz Capitulino , Markus Armbruster , Pavel Butsykin On 10/15/2015 08:23 PM, Alex Bennée wrote: > 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 as far as I can see that patch is one year old at least and my version is slightly better, it does the same for qemu_log. The difference is that old patch has unlikely() hint and does not contain break. I can revert the condition and add the hint in a couple of minutes if this will increase the chance to get both things merged. We should have both functions as macros. Will you accept that? Den