From patchwork Tue Nov 12 15:01:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robert Foley X-Patchwork-Id: 179210 Delivered-To: patch@linaro.org Received: by 2002:a92:38d5:0:0:0:0:0 with SMTP id g82csp8238490ilf; Tue, 12 Nov 2019 07:05:11 -0800 (PST) X-Google-Smtp-Source: APXvYqzkvbey6aPUZ9NpKcICjfKi5gOHfr0/0B5UiPz6r++GA3OUZJ20ZvQ43i2EQUUh6C8Ycfwb X-Received: by 2002:a50:959a:: with SMTP id w26mr33350950eda.214.1573571110781; Tue, 12 Nov 2019 07:05:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1573571110; cv=none; d=google.com; s=arc-20160816; b=LcLvn633bozhjv/Rzj23fN7k/VghheUx+lafLa8BwyVYfOEx5KXt609wrbufee3rfL hA9iQSQ1Qw0565ZOQ3Ob1CmEDAkX+U2tjcBjP8ORst/8FVmyxHBUZSdt+Iz+j3JxFKzL IxN6+2svjj2SDq7jgtl6wC4yl3lO+IjnGfGPp+FqsnURO5CnGLyNT5JTnO5JIjJ44bOv vqL/Ng2v/Uhd9jQlIvWJBNDf7CtA4NqHHTldDiJLiNsYc1wDsOPiI3Eg/C1OeShjZt05 VtrXcaxaVGBFCB02fWjRNGBgbCH1pJPV/jFLhSJvgD47m8NMZbYwwdhyfL+3XDOstitk OHVA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:references:in-reply-to :message-id:date:subject:to:from:dkim-signature; bh=G2Eghhl+NgguGFeyOq1jUpx4gOBZtmlNpBTN4T0iFt0=; b=GA1hUjXDsbtW5yLPWZrNMmDBDR6f0DpXGRkwymCdZY2fHk2/sSSKGZasErcQY2k9Bc 62oouXJ81Gh4WCBPA/z5dtGESGMpdPkvtf9WhXlsR63enPXYaO7xsCtkcEsyk+xqOVW0 XIm6IbGMqL/e9hdpcHkQMtkvegGJeGIBbJH9FoTjk8gwoz32L5gH2KF59dIQkMDexhlQ Usq6lzjPSRclHUoT1ndCdSdu3SolP5XtGQ+jiVZXXxBdohDLjdR9eynDoB4Fym9RjbOt VYfoZvbvSLXFsAL5yhuot3t1MhQfaxLQ6k+1xUZ6WYLrB48i+50b3hnGFcffxROKgsBp Ed7Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=WS3AVuXy; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id s13si131164ejx.144.2019.11.12.07.05.10 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 12 Nov 2019 07:05:10 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=WS3AVuXy; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:36146 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iUXjB-0000DD-Ce for patch@linaro.org; Tue, 12 Nov 2019 10:05:09 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:38035) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iUXge-0005xW-9T for qemu-devel@nongnu.org; Tue, 12 Nov 2019 10:02:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iUXgb-0002J6-FC for qemu-devel@nongnu.org; Tue, 12 Nov 2019 10:02:32 -0500 Received: from mail-pl1-x643.google.com ([2607:f8b0:4864:20::643]:39014) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iUXgb-0002I4-4R for qemu-devel@nongnu.org; Tue, 12 Nov 2019 10:02:29 -0500 Received: by mail-pl1-x643.google.com with SMTP id o9so9494222plk.6 for ; Tue, 12 Nov 2019 07:02:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=G2Eghhl+NgguGFeyOq1jUpx4gOBZtmlNpBTN4T0iFt0=; b=WS3AVuXyss2Jc19DDI4XR3K9NCPQjBYFoHekFkzYnlre2KPp2u10Ub0P+ep6cpwTGE AiiCwtAhIRSAHHj52qO5nUgjpZXxXSCOMDIB0hho1861vSMlc0A5LtkJHaHTGVVo1oCW yCnlgUa55gG3HE6+5lsC6wz0saFpaD3GPbGCmEMp4anMP3eBypJKziG3QYx6WJJXn2Wo +g0z0hQZtV/ehwfbmTFk6sCwV4M3YMu68N0+9E7X8x7l9VYJLLUeHex2Z1/CRLz91m6U eHG1k4F4gW6qkP1+HDsovspJU1rkxfzPG877/BEifv8cCXaJI0wQMotaovm/6J6MEtKI ZYlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=G2Eghhl+NgguGFeyOq1jUpx4gOBZtmlNpBTN4T0iFt0=; b=PzbV+Pu2u9xQkfWBnNlm/kGvvVTXdk8kO4ArzyPfe8S9L7yOFMsvx9MC0oDpIfUDnj tof7ZAOe/2QyfqoFA3+S3VB+Gq+ZkqOOK/ycZY35Q3wwlZu0AeU61hSDl4V7tmQ+F5sW 87ZthiOtikQlqlZFvPyYJijxHxPAvGLmQYcsp5k5sAk/GNbsNCV/uBu9LAq7sF3fTOlY bhBCP4K6QVHF/0KFv6ZPvmIvOxEswIZ+wHEzuc+jh6oASN3rhNN3eWS+pTLFZi7N5mJ3 EBtxKUM0JzVMMoHG9aXsiQAPxZyQGkR8QvFqXtyWl57jsFTiNgbMUPQP//GaSFX4Mfmv ra8w== X-Gm-Message-State: APjAAAXXbAtptos5qQb0VJtkdcCtE/R1QD0Z9xjsuXCCSdOZMLrUnkmQ +HaoCsIp9gvlVKY+IZ6nC9EAgHSexLc= X-Received: by 2002:a17:902:6acb:: with SMTP id i11mr32594190plt.214.1573570947606; Tue, 12 Nov 2019 07:02:27 -0800 (PST) Received: from Rfoley-MA01.usrd.futurewei.com ([12.111.81.71]) by smtp.gmail.com with ESMTPSA id a3sm8235511pfo.71.2019.11.12.07.02.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Nov 2019 07:02:27 -0800 (PST) From: Robert Foley To: qemu-devel@nongnu.org Subject: [PATCH v1 3/5] Add use of RCU for qemu_logfile. Date: Tue, 12 Nov 2019 10:01:03 -0500 Message-Id: <20191112150105.2498-4-robert.foley@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20191112150105.2498-1-robert.foley@linaro.org> References: <20191112150105.2498-1-robert.foley@linaro.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::643 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.puhov@linaro.org, alex.bennee@linaro.org, robert.foley@linaro.org Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" This now allows changing the logfile while logging is active, and also solves the issue of a seg fault while changing the logfile. Any read access to the qemu_logfile handle will use the rcu_read_lock()/unlock() around the use of the handle. To fetch the handle we will use atomic_rcu_read(). We also in many cases do a check for validity of the logfile handle before using it to deal with the case where the file is closed and set to NULL. The cases where we write to the qemu_logfile will use atomic_rcu_set(). Writers will also use call_rcu() with a newly added qemu_logfile_free function for freeing/closing when readers have finished. Signed-off-by: Robert Foley --- v1 - Changes for review comments. - Minor changes to definition of QemuLogFile. - Changed qemu_log_separate() to fix unbalanced and remove qemu_log_enabled() check. - changed qemu_log_lock() to include else. - make qemu_logfile_free static. - use g_assert(logfile) in qemu_logfile_free. - Relocated unlock out of if/else in qemu_log_close(), and in qemu_set_log(). --- include/qemu/log.h | 42 ++++++++++++++++++++++---- util/log.c | 73 +++++++++++++++++++++++++++++++++------------- include/exec/log.h | 33 ++++++++++++++++++--- tcg/tcg.c | 12 ++++++-- 4 files changed, 128 insertions(+), 32 deletions(-) -- 2.17.1 diff --git a/include/qemu/log.h b/include/qemu/log.h index a7c5b01571..528e1f9dd7 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -3,9 +3,16 @@ /* A small part of this API is split into its own header */ #include "qemu/log-for-trace.h" +#include "qemu/rcu.h" + +typedef struct QemuLogFile { + struct rcu_head rcu; + FILE *fd; +} QemuLogFile; /* Private global variable, don't use */ -extern FILE *qemu_logfile; +extern QemuLogFile *qemu_logfile; + /* * The new API: @@ -25,7 +32,16 @@ static inline bool qemu_log_enabled(void) */ static inline bool qemu_log_separate(void) { - return qemu_logfile != NULL && qemu_logfile != stderr; + QemuLogFile *logfile; + bool res = false; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile && logfile->fd != stderr) { + res = true; + } + rcu_read_unlock(); + return res; } #define CPU_LOG_TB_OUT_ASM (1 << 0) @@ -55,14 +71,23 @@ static inline bool qemu_log_separate(void) static inline FILE *qemu_log_lock(void) { - qemu_flockfile(qemu_logfile); - return logfile->fd; + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + qemu_flockfile(logfile->fd); + return logfile->fd; + } else { + rcu_read_unlock(); + return NULL; + } } static inline void qemu_log_unlock(FILE *fd) { if (fd) { qemu_funlockfile(fd); + rcu_read_unlock(); } } @@ -73,9 +98,14 @@ static inline void qemu_log_unlock(FILE *fd) static inline void GCC_FMT_ATTR(1, 0) qemu_log_vprintf(const char *fmt, va_list va) { - if (qemu_logfile) { - vfprintf(qemu_logfile, fmt, va); + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + vfprintf(logfile->fd, fmt, va); } + rcu_read_unlock(); } /* log only if a bit is set on the current loglevel mask: diff --git a/util/log.c b/util/log.c index c25643dc99..802b8de42e 100644 --- a/util/log.c +++ b/util/log.c @@ -28,7 +28,7 @@ static char *logfilename; static QemuMutex qemu_logfile_mutex; -FILE *qemu_logfile; +QemuLogFile *qemu_logfile; int qemu_loglevel; static int log_append = 0; static GArray *debug_regions; @@ -37,10 +37,14 @@ static GArray *debug_regions; int qemu_log(const char *fmt, ...) { int ret = 0; - if (qemu_logfile) { + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { va_list ap; va_start(ap, fmt); - ret = vfprintf(qemu_logfile, fmt, ap); + ret = vfprintf(logfile->fd, fmt, ap); va_end(ap); /* Don't pass back error results. */ @@ -48,6 +52,7 @@ int qemu_log(const char *fmt, ...) ret = 0; } } + rcu_read_unlock(); return ret; } @@ -56,11 +61,23 @@ static void __attribute__((__constructor__)) qemu_logfile_init(void) qemu_mutex_init(&qemu_logfile_mutex); } +static void qemu_logfile_free(QemuLogFile *logfile) +{ + g_assert(logfile); + + if (logfile->fd != stderr) { + fclose(logfile->fd); + } + g_free(logfile); +} + static bool log_uses_own_buffers; /* enable or disable low levels log */ void qemu_set_log(int log_flags) { + QemuLogFile *logfile; + qemu_loglevel = log_flags; #ifdef CONFIG_TRACE_LOG qemu_loglevel |= LOG_TRACE; @@ -70,44 +87,50 @@ void qemu_set_log(int log_flags) qemu_mutex_lock(&qemu_logfile_mutex); if (!qemu_logfile && (is_daemonized() ? logfilename != NULL : qemu_loglevel)) { + logfile = g_new0(QemuLogFile, 1); if (logfilename) { - qemu_logfile = fopen(logfilename, log_append ? "a" : "w"); - if (!qemu_logfile) { + logfile->fd = fopen(logfilename, log_append ? "a" : "w"); + if (!logfile->fd) { + g_free(logfile); perror(logfilename); _exit(1); } /* In case we are a daemon redirect stderr to logfile */ if (is_daemonized()) { - dup2(fileno(qemu_logfile), STDERR_FILENO); - fclose(qemu_logfile); + dup2(fileno(logfile->fd), STDERR_FILENO); + fclose(logfile->fd); /* This will skip closing logfile in qemu_log_close() */ - qemu_logfile = stderr; + logfile->fd = stderr; } } else { /* Default to stderr if no log file specified */ assert(!is_daemonized()); - qemu_logfile = stderr; + logfile->fd = stderr; } /* must avoid mmap() usage of glibc by setting a buffer "by hand" */ if (log_uses_own_buffers) { static char logfile_buf[4096]; - setvbuf(qemu_logfile, logfile_buf, _IOLBF, sizeof(logfile_buf)); + setvbuf(logfile->fd, logfile_buf, _IOLBF, sizeof(logfile_buf)); } else { #if defined(_WIN32) /* Win32 doesn't support line-buffering, so use unbuffered output. */ - setvbuf(qemu_logfile, NULL, _IONBF, 0); + setvbuf(logfile->fd, NULL, _IONBF, 0); #else - setvbuf(qemu_logfile, NULL, _IOLBF, 0); + setvbuf(logfile->fd, NULL, _IOLBF, 0); #endif log_append = 1; } + atomic_rcu_set(&qemu_logfile, logfile); } - qemu_mutex_unlock(&qemu_logfile_mutex); + logfile = qemu_logfile; + if (qemu_logfile && (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) { - qemu_log_close(); + atomic_rcu_set(&qemu_logfile, NULL); + call_rcu(logfile, qemu_logfile_free, rcu); } + qemu_mutex_unlock(&qemu_logfile_mutex); } void qemu_log_needs_buffers(void) @@ -123,6 +146,7 @@ void qemu_log_needs_buffers(void) void qemu_set_log_filename(const char *filename, Error **errp) { char *pidstr; + g_free(logfilename); pidstr = strstr(filename, "%"); @@ -235,19 +259,28 @@ out: /* fflush() the log file */ void qemu_log_flush(void) { - fflush(qemu_logfile); + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + fflush(logfile->fd); + } + rcu_read_unlock(); } /* Close the log file */ void qemu_log_close(void) { + QemuLogFile *logfile; + g_assert(qemu_logfile_mutex.initialized); qemu_mutex_lock(&qemu_logfile_mutex); - if (qemu_logfile) { - if (qemu_logfile != stderr) { - fclose(qemu_logfile); - } - qemu_logfile = NULL; + logfile = qemu_logfile; + + if (logfile) { + atomic_rcu_set(&qemu_logfile, NULL); + call_rcu(logfile, qemu_logfile_free, rcu); } qemu_mutex_unlock(&qemu_logfile_mutex); } diff --git a/include/exec/log.h b/include/exec/log.h index e2cfd436e6..9bd1e4aa20 100644 --- a/include/exec/log.h +++ b/include/exec/log.h @@ -15,8 +15,15 @@ */ static inline void log_cpu_state(CPUState *cpu, int flags) { + QemuLogFile *logfile; + if (qemu_log_enabled()) { - cpu_dump_state(cpu, qemu_logfile, flags); + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + cpu_dump_state(cpu, logfile->fd, flags); + } + rcu_read_unlock(); } } @@ -40,19 +47,37 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags) static inline void log_target_disas(CPUState *cpu, target_ulong start, target_ulong len) { - target_disas(qemu_logfile, cpu, start, len); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + target_disas(logfile->fd, cpu, start, len); + } + rcu_read_unlock(); } static inline void log_disas(void *code, unsigned long size) { - disas(qemu_logfile, code, size); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + disas(logfile->fd, code, size); + } + rcu_read_unlock(); } #if defined(CONFIG_USER_ONLY) /* page_dump() output to the log file: */ static inline void log_page_dump(void) { - page_dump(qemu_logfile); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + page_dump(logfile->fd); + } + rcu_read_unlock(); } #endif #endif diff --git a/tcg/tcg.c b/tcg/tcg.c index 0511266d85..4f616ba38b 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -2114,9 +2114,17 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs) } if (have_prefs || op->life) { - for (; col < 40; ++col) { - putc(' ', qemu_logfile); + + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + for (; col < 40; ++col) { + putc(' ', logfile->fd); + } } + rcu_read_unlock(); } if (op->life) {