[ltt-dev] [UST PATCH] UST synchronization fix

[chris meyers]

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 <mathieu.desnoyers at efficios.com>
> > > > > ---
> > > > >  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: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20100812/5d843675/attachment-0003.htm>