From patchwork Wed Feb 6 15:30:14 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 218675 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4F2F62C008D for ; Thu, 7 Feb 2013 02:31:19 +1100 (EST) Received: from localhost ([::1]:48752 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U36xx-00020v-DO for incoming@patchwork.ozlabs.org; Wed, 06 Feb 2013 10:31:17 -0500 Received: from eggs.gnu.org ([208.118.235.92]:49883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U36xh-0001oz-LH for qemu-devel@nongnu.org; Wed, 06 Feb 2013 10:31:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1U36xe-0007pq-5F for qemu-devel@nongnu.org; Wed, 06 Feb 2013 10:31:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32136) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1U36xd-0007pl-UQ for qemu-devel@nongnu.org; Wed, 06 Feb 2013 10:30:58 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r16FUqRv009283 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 6 Feb 2013 10:30:57 -0500 Received: from localhost (ovpn-112-20.ams2.redhat.com [10.36.112.20]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r16FUeeM016109; Wed, 6 Feb 2013 10:30:42 -0500 From: Stefan Hajnoczi To: Date: Wed, 6 Feb 2013 16:30:14 +0100 Message-Id: <1360164617-12103-2-git-send-email-stefanha@redhat.com> In-Reply-To: <1360164617-12103-1-git-send-email-stefanha@redhat.com> References: <1360164617-12103-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Anthony Liguori , Markus Armbruster , Stefan Hajnoczi Subject: [Qemu-devel] [PATCH 1/4] trace: Fix simple trace dropped event record for big endian X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Markus Armbruster We use atomic operations to keep track of dropped events. Inconveniently, GLib supports only int and void * atomics, but the counter dropped_events is uint64_t. Can't stop commit 62bab732: a quick (gint *)&dropped_events bludgeons the compiler into submission. That cast is okay only when int is exactly 64 bits wide, which it commonly isn't. If int is even wider, we clobber whatever follows dropped_events. Not worth worrying about, as none of the machines that interest us have such morbidly obese ints. That leaves the common case: int narrower than 64 bits. Harmless on little endian hosts: we just don't access the most significant bits of dropped_events. They remain zero. On big endian hosts, we use only the most significant bits of dropped_events as counter. The least significant bits remain zero. However, we write out the full value, which is the correct counter shifted left a bunch of places. Fix by changing the variables involved to int. There's another, equally suspicious-looking (gint *)&trace_idx argument to g_atomic_int_compare_and_exchange(), but that one casts unsigned *, so it's okay. But it's also superfluous, because GLib's atomic int operations work just fine for unsigned. Drop it. Signed-off-by: Markus Armbruster Reviewed-by: Laszlo Ersek Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index ce17d64..ccbdb6a 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -53,7 +53,7 @@ enum { uint8_t trace_buf[TRACE_BUF_LEN]; static unsigned int trace_idx; static unsigned int writeout_idx; -static uint64_t dropped_events; +static int dropped_events; static FILE *trace_fp; static char *trace_file_name; @@ -63,7 +63,7 @@ typedef struct { uint64_t timestamp_ns; uint32_t length; /* in bytes */ uint32_t reserved; /* unused */ - uint8_t arguments[]; + uint64_t arguments[]; } TraceRecord; typedef struct { @@ -160,7 +160,7 @@ static gpointer writeout_thread(gpointer opaque) uint8_t bytes[sizeof(TraceRecord) + sizeof(uint64_t)]; } dropped; unsigned int idx = 0; - uint64_t dropped_count; + int dropped_count; size_t unused __attribute__ ((unused)); for (;;) { @@ -169,16 +169,16 @@ static gpointer writeout_thread(gpointer opaque) if (dropped_events) { dropped.rec.event = DROPPED_EVENT_ID, dropped.rec.timestamp_ns = get_clock(); - dropped.rec.length = sizeof(TraceRecord) + sizeof(dropped_events), + dropped.rec.length = sizeof(TraceRecord) + sizeof(uint64_t), dropped.rec.reserved = 0; while (1) { dropped_count = dropped_events; - if (g_atomic_int_compare_and_exchange((gint *)&dropped_events, + if (g_atomic_int_compare_and_exchange(&dropped_events, dropped_count, 0)) { break; } } - memcpy(dropped.rec.arguments, &dropped_count, sizeof(uint64_t)); + dropped.rec.arguments[0] = dropped_count; unused = fwrite(&dropped.rec, dropped.rec.length, 1, trace_fp); } @@ -220,11 +220,11 @@ int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasi if (new_idx - writeout_idx > TRACE_BUF_LEN) { /* Trace Buffer Full, Event dropped ! */ - g_atomic_int_inc((gint *)&dropped_events); + g_atomic_int_inc(&dropped_events); return -ENOSPC; } - if (g_atomic_int_compare_and_exchange((gint *)&trace_idx, + if (g_atomic_int_compare_and_exchange(&trace_idx, old_idx, new_idx)) { break; }