From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42501) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfOzd-0006Mh-KD for qemu-devel@nongnu.org; Fri, 15 Jun 2012 01:22:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfOzb-0008AH-Ks for qemu-devel@nongnu.org; Fri, 15 Jun 2012 01:22:45 -0400 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:40482) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfOza-00088T-TZ for qemu-devel@nongnu.org; Fri, 15 Jun 2012 01:22:43 -0400 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 15 Jun 2012 10:52:36 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q5F5MXwE1114622 for ; Fri, 15 Jun 2012 10:52:33 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q5FApnVS024781 for ; Fri, 15 Jun 2012 20:51:49 +1000 Message-ID: <4FDAC693.2020909@linux.vnet.ibm.com> Date: Fri, 15 Jun 2012 10:52:27 +0530 From: Harsh Bora MIME-Version: 1.0 References: <1339699146-26225-1-git-send-email-harsh@linux.vnet.ibm.com> <1339699146-26225-3-git-send-email-harsh@linux.vnet.ibm.com> In-Reply-To: <1339699146-26225-3-git-send-email-harsh@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 2/3] Simpletrace v2: Support multiple arguments, strings. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Harsh Prateek Bora Cc: stefanha@gmail.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On 06/15/2012 12:09 AM, Harsh Prateek Bora wrote: > static gpointer writeout_thread(gpointer opaque) > { > - TraceRecord record; > - unsigned int writeout_idx = 0; > - unsigned int num_available, idx; > + TraceRecord *recordptr, *dropped_ptr; > + union { > + TraceRecord rec; > + uint8_t bytes[sizeof(TraceRecord) + sizeof(uint64_t)]; > + } dropped; > + dropped_ptr = (TraceRecord *)dropped.bytes; > + unsigned int idx = 0; > + uint64_t dropped_count; > size_t unused __attribute__ ((unused)); > > for (;;) { > wait_for_trace_records_available(); > > - num_available = trace_idx - writeout_idx; > - if (num_available> TRACE_BUF_LEN) { > - record = (TraceRecord){ > - .event = DROPPED_EVENT_ID, > - .x1 = num_available, > - }; > - unused = fwrite(&record, sizeof(record), 1, trace_fp); > - writeout_idx += num_available; > + if (dropped_events) { > + dropped_ptr->event = DROPPED_EVENT_ID, > + dropped_ptr->timestamp_ns = get_clock(); > + dropped_ptr->length = sizeof(TraceRecord) + sizeof(dropped_events), > + dropped_ptr->reserved = 0; > + while (1) { > + dropped_count = dropped_events; > + if (g_atomic_int_compare_and_exchange((gint *)&dropped_events, > + dropped_count, 0)) { > + break; > + } > + } > + memcpy(dropped_ptr->arguments,&dropped_count, sizeof(uint64_t)); > + unused = fwrite(dropped_ptr, dropped_ptr->length, 1, trace_fp); > } > > - idx = writeout_idx % TRACE_BUF_LEN; > - while (get_trace_record(idx,&record)) { > - trace_buf[idx].event = 0; /* clear valid bit */ > - unused = fwrite(&record, sizeof(record), 1, trace_fp); > - idx = ++writeout_idx % TRACE_BUF_LEN; > + while (get_trace_record(idx,&recordptr)) { > + unused = fwrite(recordptr, recordptr->length, 1, trace_fp); > + writeout_idx += recordptr->length; > + g_free(recordptr); > } > + /* Note: The idx assignment expression below, > + * if kept in while-loop above, results in tracelog > + * corruption, possibly due to compiler-reordering of > + * statements. Keeping it out of loop saves a memory barrier. > + */ Hmm, I tried moving this assignment to while-loop using memory barrier before it, to ensure the loop can read multiple records without coming out of loop, however its still giving me corrupted records. However, keeping the assignment out of loop always works for me. Not sure what could be the reason for such a behaviour, above comment thus needs an update. ~ Harsh > + idx = writeout_idx % TRACE_BUF_LEN; > > fflush(trace_fp); > } > return NULL; > }