From mboxrd@z Thu Jan 1 00:00:00 1970 From: chris.meyers.fsu@gmail.com (chris meyers) Date: Thu, 12 Aug 2010 15:37:10 -0400 Subject: [ltt-dev] [UST PATCH] UST synchronization fix In-Reply-To: <20100812184202.GA26342@Krystal> References: <20100811141342.GA24267@Krystal> <20100811155627.GA24570@Krystal> <20100812184202.GA26342@Krystal> Message-ID: Message-ID: <20100812193710.jTBxrd8UJoHWPtMBE23dzmSq0mHDltumHr9UDDbouw4@z> Awesome! It felt like the marker didn't match up with the data somehow. Thanks for working on this problem! -Chris On Thu, Aug 12, 2010 at 2:42 PM, Mathieu Desnoyers < mathieu.desnoyers at efficios.com> wrote: > Oh, Sh*t..... I found the underlying problem. > > in lttv: > ltt/marker.c > > marker_update_fields_offsets() updates the marker info offset value, which > affects the offset for the marker for the whole trace. However, this > information > should stay valid as long as the event is "current" in a given tracefile. > > So the problem we have here is that we have the same marker twice in two > threads, with one tracefile per thread. The offsets gets corrupted by > events > read from the other thread while we have the first thread read traceset > middle > reading the event at the lowest level and the event eventually dispatched > (in > order) to the text dump module. > > I'll look into this and come back with a fix. > > Thanks, > > Mathieu > > * chris meyers (chris.meyers.fsu at gmail.com) wrote: > > I can still invoke the problem with the modified test case that you sent > me. > > It works flawlessly without the sleeps, but when I put the sleeps back > in > > it garbles the output. Did it run OK with the sleeps in for you? > > > > -Chris > > > > > > On Wed, Aug 11, 2010 at 11:56 AM, Mathieu Desnoyers < > > mathieu.desnoyers at efficios.com> wrote: > > > > > No. But the test-case was incorrect: the string input was concurrently > > > modified > > > by the 2 threads. I'm preparing modifications to string handling to be > more > > > robust (so the trace does not become unreadable). > > > > > Ahh, my bad. That was a mistake to have the different threads modifying > the > > same string. Thanks for the fixed version. > > > > > > > > In your original problem, did you have another event that was trying to > > > save > > > concurrently modified strings into the trace buffers ? > > > > > No > > > > > > > > It's a 2-pass algo: one pass for strlen, 2nd pass doing a memcpy of the > > > string. > > > If it gets modified concurrently, then bad things happen. > > > > > > Thanks, > > > > > > Mathieu > > > > > > * chris meyers (chris.meyers.fsu at gmail.com) wrote: > > > > Did this fix solve the multi-threaded test case for you? > > > > > > > > Thanks, > > > > -Chris > > > > > > > > On Wed, Aug 11, 2010 at 10:13 AM, Mathieu Desnoyers < > > > > mathieu.desnoyers at efficios.com> wrote: > > > > > > > > > Missing smp_wmb() at subbuffer switch. > > > > > > > > > > Signed-off-by: Mathieu Desnoyers > > > > > --- > > > > > git/ust/libust/buffers.c | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > Index: ust/libust/buffers.c > > > > > =================================================================== > > > > > --- ust/libust/buffers.c 2010-08-10 16:16:39.000000000 -0400 > > > > > +++ ust/libust/buffers.c 2010-08-10 16:19:59.000000000 -0400 > > > > > @@ -858,7 +858,7 @@ static void ltt_reserve_switch_old_subbu > > > > > * This compiler barrier is upgraded into a smp_wmb() by the > IPI > > > > > * sent by get_subbuf() when it does its smp_rmb(). > > > > > */ > > > > > - barrier(); > > > > > + smp_wmb(); > > > > > uatomic_add(&buf->commit_count[oldidx].cc, padding_size); > > > > > commit_count = uatomic_read(&buf->commit_count[oldidx].cc); > > > > > ltt_check_deliver(chan, buf, offsets->old - 1, commit_count, > > > > > oldidx); > > > > > @@ -887,7 +887,7 @@ static void ltt_reserve_switch_new_subbu > > > > > * This compiler barrier is upgraded into a smp_wmb() by the > IPI > > > > > * sent by get_subbuf() when it does its smp_rmb(). > > > > > */ > > > > > - barrier(); > > > > > + smp_wmb(); > > > > > uatomic_add(&buf->commit_count[beginidx].cc, > > > > > ltt_subbuffer_header_size()); > > > > > commit_count = > uatomic_read(&buf->commit_count[beginidx].cc); > > > > > /* Check if the written buffer has to be delivered */ > > > > > @@ -932,7 +932,7 @@ static void ltt_reserve_end_switch_curre > > > > > * This compiler barrier is upgraded into a smp_wmb() by the > IPI > > > > > * sent by get_subbuf() when it does its smp_rmb(). > > > > > */ > > > > > - barrier(); > > > > > + smp_wmb(); > > > > > uatomic_add(&buf->commit_count[endidx].cc, padding_size); > > > > > commit_count = uatomic_read(&buf->commit_count[endidx].cc); > > > > > ltt_check_deliver(chan, buf, > > > > > -- > > > > > Mathieu Desnoyers > > > > > Operating System Efficiency R&D Consultant > > > > > EfficiOS Inc. > > > > > http://www.efficios.com > > > > > > > > > > _______________________________________________ > > > > > ltt-dev mailing list > > > > > ltt-dev at lists.casi.polymtl.ca > > > > > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > > > > > > > > > > > -- > > > Mathieu Desnoyers > > > Operating System Efficiency R&D Consultant > > > EfficiOS Inc. > > > http://www.efficios.com > > > > > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com > -------------- next part -------------- An HTML attachment was scrubbed... URL: