Re: [lttng-dev] [PATCH 00/11] Add support for TSAN to liburcu - Dmitry Vyukov via lttng-dev

From: Dmitry Vyukov via lttng-dev <lttng-dev@lists.lttng.org>
To: "Ondřej Surý" <ondrej@sury.org>
Cc: Olivier Dion <odion@efficios.com>,
	tsan-users <tsan-users@google.com>,
	lttng-dev@lists.lttng.org,
	 "Paul E. McKenney" <paulmck@kernel.org>,
	Tony Finch <fanf@isc.org>
Subject: Re: [lttng-dev] [PATCH 00/11] Add support for TSAN to liburcu
Date: Fri, 26 May 2023 08:08:40 +0200	[thread overview]
Message-ID: <CACT4Y+bwGm1h5A0qsjCQrLzwt+uAbNZ-D+6zSr6PiXDGq2LgYw@mail.gmail.com> (raw)
In-Reply-To: <EFA4F8F7-0767-4FEF-8EA5-DDEA05F3296C@sury.org>

On Fri, 26 May 2023 at 07:33, Ondřej Surý <ondrej@sury.org> wrote:
>
> A little bit related question/problem - some of our system test jobs got significantly slower (3x-5x) with **clang-16** TSAN after we integrated liburcu to BIND 9. The GCC (13.1.1) TSAN doesn’t exhibit this behavior.
>
> The liburcu integration in BIND 9 is very light at the moment - we use liburcu for qptrie and there’s some usage of wfcqueue for queuing jobs(callbacks) between threads.
>
> We were not able to put a finger on it yet, but saying this aloud might help diagnose this.

Interesting. Clang 16 got a completely new tsan runtime
implementation, which is generally ~2x faster and consumes less
memory.
However, of course, it's not possible to optimize for all gazillion of
programs in the world. I suspect librcu tests are quite extreme in
terms of synchronization.

If you collect perf profiles for both, I can look for some low hanging fruit.
Or is there a reasonable way for me to reproduce?

> Ondrej
> --
> Ondřej Surý <ondrej@sury.org> (He/Him)
>
> > On 24. 5. 2023, at 10:15, Dmitry Vyukov via lttng-dev <lttng-dev@lists.lttng.org> wrote:
> >
> > On Tue, 23 May 2023 at 18:05, Olivier Dion <odion@efficios.com> wrote:
> >>
> >> Hi Dmitry,
> >>
> >> We do have a new issue and we think it might be a limitation from TSAN.
> >>
> >> Find attached a test program that we believe is correct.  You can
> >> compile it with `gcc -fsanitize=thread test.c -pthread'.
> >>
> >>
> >> TSAN flags a race condition between the atomic store relaxed at line 36
> >> and the free at line 81.
> >>
> >> We can solve the issue by replacing the atomic store relaxed with a
> >> atomic store release.  However, the preceding atomic exchange with
> >> sequential consistency already acts as an implicit release (in term of
> >> memory barrier) for the following store, making the release semantic
> >> redundant.
> >>
> >> We've found an alternative to fix this by ignoring the thread during the
> >> relaxed store (line 39).  However, what we would like is to annotate the
> >> memory stored (line 45).
> >>
> >> My theory (I don't know the internal of TSAN much) is that TSAN thinks
> >> for some reason that the atomic store relaxed happen at the same epoch
> >> as the free, resulting in a false positive.  If so, m
> >
> >
> > I don't this this is true in the C/C++ memory model:
> > "the preceding atomic exchange with sequential consistency already
> > acts as an implicit release (in term of memory barrier) for the
> > following store".
> >
> > std::atomic_thread_fence does affect all preceding/subsequent
> > operations, but an atomic memory operation only affects ordering on
> > that variable, it doesn't also serve as a standalone memory fence.
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev