Re: URCU feature request?
Thobias Knudsen via lttng-dev <[email protected]> Fri, 19 Sep 2025 22:39:11 +0200
| Newsgroups | org.lttng.lists.lttng-dev |
|---|---|
| Message-ID | <CAKGpcio7tO-G0JBazV1Xd52==uf9DCtxPfeUtne_QnNivCKacw@mail.gmail.com> |
--0000000000000b9882063f2d7704
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
>Since 0.15.0, we've introduce an annotation layer (not part of the
>public API), making TSAN compatible with URCU. See
>`include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD' macro.
>
>However, IIRC, you also need to compile URCU with the configuration
>option `--enable-compiler-atomic-builtins' so that atomic operations are
>implemented with the configured toolchain's builtin atomics. I don't
>recall if this was strictly necessary.
>
>If you encounter false positives with TSAN, please send me a minimal
>reproducible example together with:
>
> - the toolchain you=E2=80=99re using
>
> - the version of URCU
>
> - the configuration flags you used
>
>I will be happy to have a look.
Sorry for the late answer. I had to find the time to fix some bugs to get
it all running before switching to urcu v0.15.0 and setting the flags and
macros for thread sanitizer support. Creating a minimum reproducible
example would take some time as I don't have any good idea of the
exact place where the thread sanitizer issues arise because they are
scattered all over tsm.c and test_tsm.c. If you have linux debian family
then you can clone my repo and run it:
https://github.com/ThobiasKnudsen/Logos. ./scripts/build.sh --debug --tsan
&& ./build/bin/test_tsm. I tried running with and without thread sanitizer
support and got the same issues. In the test_tsm when only one thread is
running, the thread sanitizer produces warnings still. I did find a place
where the thread sanitizer said there was a race and the write was within a
node not yet inserted into the cds_lfht. That seems like a false positive.
The read for the same case should be inside rcu_read_lock as everything
within lines 263 to 904 inside test_tsm.c is within a read section.
Read of size 1 at 0x7b1400085ea1 by thread T6:
#0 tsm_base_node_is_valid /Logos/src/tsm.c:721 (test_tsm+0xa548)
#1 _tsm_tsm_type_is_valid /Logos/src/tsm.c:364 (test_tsm+0xb1bf)
#2 tsm_node_is_valid /Logos/src/tsm.c:1553 (test_tsm+0x9984)
#3 tsm_node_defer_free /Logos/src/tsm.c:1888 (test_tsm+0xb2fc)
#4 stress_thread /Logos/src/tests/test_tsm.c:549 (test_tsm+0x46a6)
Previous write of size 1 at 0x7b1400085ea1 by thread T9:
#0 tsm_base_node_create /Logos/src/tsm.c:683 (test_tsm+0x6f2a)
#1 stress_thread /Logos/src/tests/test_tsm.c:843 (test_tsm+0x5ce0)
>I don=E2=80=99t see why that would be a problem for the static-check algor=
ithm I
>described above. If a pointer needs to be protected by a mutex for
>mutation, that falls outside the scope of RCU, as far as I know.
If I understand correctly the __rcu checks that reads are not done outside
read sections and unsafe writes are not done at all. If this is correct
then if you are using custom concurrency for __rcu protected data outside
the read section or unsafe writes is that allowed? Because if it's not that
would be a limitation for __rcu.
man. 8. sep. 2025 kl. 02:10 skrev Olivier Dion <[email protected]>:
> On Sun, 07 Sep 2025, Thobias Knudsen <[email protected]> wrote:
> >> It looks like you want runtime verification for the usage of the API.
> >> Did you know that URCU can now be compiled against ThreadSanitizer
> >> (TSAN)? If a user misuses the API or makes incorrect assumptions abou=
t
> >> the guarantees offered by RCU, TSAN will most likely detect those
> >> issues. Coupled with the other debug features we already have, this
> >> makes it very hard to not trigger an error path when the API is used
> >> incorrectly.
> >
> > Really?! I've used TSAN and got a bunch of false positives, I believe,
> but
> > maybe they're not false positives? How do you remove the false positive=
s,
> > or know that they're not false positives?
>
> Since 0.15.0, we've introduce an annotation layer (not part of the
> public API), making TSAN compatible with URCU. See
> `include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD' macro.
>
> However, IIRC, you also need to compile URCU with the configuration
> option `--enable-compiler-atomic-builtins' so that atomic operations are
> implemented with the configured toolchain's builtin atomics. I don't
> recall if this was stricly necessary.
>
> If you encounter false positives with TSAN, please send me a minimal
> reproducible example together with:
>
> - the toolchain you=E2=80=99re using
>
> - the version of URCU
>
> - the configuration flags you used
>
> I will be happy to have a look.
>
> >> Note that certain kind of errors could actually be flag at compile tim=
e
> >> with the proper tooling. For example, the Linux kernel uses a `__rcu'
> >> attribute that Sparse can understand to flag improper use of
> >> RCU=E2=80=91protected pointers. I=E2=80=99d be very open to exposing =
something similar
> >> (an attribute) for static checkers.
> >
> > wow thanks for the info! I knew compile time checks would be possible b=
ut
> > requiring compiler operability which is a higher hanging fruit for me.
>
> I don=E2=80=99t know the details of `__rcu' from the Linux kernel. I thin=
k it=E2=80=99s
> just a macro that expands to nothing by default, but Sparse treats it as
> an attribute. I=E2=80=99m not sure exactly what checks Sparse performs w=
ith it,
> but I suspect it involves traversing the program=E2=80=99s control-flow g=
raph
> (CFG), ensuring that pointers marked with the `__rcu' qualifier are:
>
> - obtained via rcu_dereference
>
> - only dereferenced under RCU lock protection
>
> > Is '__rcu' compatible with custom concurrency? For example
> > rcu_dereference a pointer then locking a mutex inside the pointer then
> > unlock read then continue using the pointer?
>
> I don=E2=80=99t see why that would be a problem for the static-check algo=
rithm I
> described above. If a pointer needs to be protected by a mutex for
> mutation, that falls outside the scope of RCU, as far as I know.
>
> > I cant come up with something usefull other than a language rework. Is
> > it much work making the __urcu attribute?
>
> I suppose not. On top of my head, it would involve adding some pointer
> qualifier and function attributes to the primitives exposed by URCU.
> Users would also need to use the pointer qualifier when working with
> RCU-protected pointers. The qualifier and the attributes would expand
> to nothing by default, letting static checkers defining them to internal
> values. I suggest you read `Documentation/RCU/rcu_dereference.rst' in
> the Linux kernel tree if you are interested.
>
> In its current state, this would not be very useful because none of the
> major compilers provide static analysis for RCU. However, implementing
> such analysis, as a plugin, wouldn=E2=80=99t be overly difficult for some=
one
> familiar with Clang or GCC, I suppose.
>
> [...]
>
> Thanks,
> Olivier
> --
> Olivier Dion
> EfficiOS Inc.
> https://www.efficios.com
>
--0000000000000b9882063f2d7704
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr">>Since 0.15.0, we've introduce an annotation layer =
(not part of the<br>>public API), making TSAN compatible with URCU.=C2=
=A0 See<br>>`include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD&#=
39; macro.<br>><br>>However, IIRC, you also need to compile URCU with=
the configuration<br>>option `--enable-compiler-atomic-builtins' so=
that atomic operations are<br>>implemented with the configured toolchai=
n's builtin atomics.=C2=A0 I don't<br>>recall if this was strict=
ly necessary.<br>><br>>If you encounter false positives with TSAN, pl=
ease send me a minimal<br>>reproducible example together with:<br>><b=
r>> - the toolchain you=E2=80=99re using<br>><br>> - the version o=
f URCU<br>><br>> - the configuration flags you used<br>><br>>I =
will be happy to have a look.<div><br></div><div>Sorry for the late answer.=
I had to find the time to fix some bugs to get it all running before switc=
hing to urcu v0.15.0 and setting the flags and macros for thread sanitizer=
=C2=A0support. Creating a minimum reproducible example would take some time=
as I don't have any good idea of the exact=C2=A0place where the thread=
sanitizer issues arise=C2=A0because they are scattered all over tsm.c and =
test_tsm.c. If you have linux debian family then you can clone my repo and =
run it:=C2=A0<a href=3D"https://github.com/ThobiasKnudsen/Logos">https://gi=
thub.com/ThobiasKnudsen/Logos</a>. ./scripts/build.sh --debug --tsan &&=
amp; ./build/bin/test_tsm. I tried running with and without thread sanitize=
r support and got the same issues. In the test_tsm when only one thread is =
running, the thread sanitizer produces warnings still. I did find a place w=
here the thread sanitizer said there=C2=A0was a race and the write was with=
in a node not yet inserted into the cds_lfht. That seems like a false posit=
ive. The read for the same case should be inside rcu_read_lock as everythin=
g within lines 263 to 904 inside test_tsm.c is within a read section.</div>=
<div><br></div><div>=C2=A0 Read of size 1 at 0x7b1400085ea1 by thread T6:<b=
r>=C2=A0 =C2=A0 #0 tsm_base_node_is_valid /Logos/src/tsm.c:721 (test_tsm+0x=
a548)<br>=C2=A0 =C2=A0 #1 _tsm_tsm_type_is_valid /Logos/src/tsm.c:364 (test=
_tsm+0xb1bf)<br>=C2=A0 =C2=A0 #2 tsm_node_is_valid /Logos/src/tsm.c:1553 (t=
est_tsm+0x9984)<br>=C2=A0 =C2=A0 #3 tsm_node_defer_free /Logos/src/tsm.c:18=
88 (test_tsm+0xb2fc)<br>=C2=A0 =C2=A0 #4 stress_thread /Logos/src/tests/tes=
t_tsm.c:549 (test_tsm+0x46a6)<br><br>=C2=A0 Previous write of size 1 at 0x7=
b1400085ea1 by thread T9:<br>=C2=A0 =C2=A0 #0 tsm_base_node_create /Logos/s=
rc/tsm.c:683 (test_tsm+0x6f2a)<br>=C2=A0 =C2=A0 #1 stress_thread /Logos/src=
/tests/test_tsm.c:843 (test_tsm+0x5ce0)<br></div><div><br></div><div>>I =
don=E2=80=99t see why that would be a problem for the static-check algorith=
m I<br>>described above.=C2=A0 If a pointer needs to be protected by a m=
utex for<br>>mutation, that falls outside the scope of RCU, as far as I =
know.</div><div><br></div><div>If I understand correctly the __rcu checks t=
hat reads are not done outside read sections and unsafe writes are not done=
at all. If this is correct then if you are using custom concurrency for __=
rcu protected data outside the read section or unsafe writes is that allowe=
d? Because if it's not that would be a limitation for __rcu.=C2=A0</div=
></div><br><div class=3D"gmail_quote gmail_quote_container"><div dir=3D"ltr=
" class=3D"gmail_attr">man. 8. sep. 2025 kl. 02:10 skrev Olivier Dion <<=
a href=3D"mailto:[email protected]">[email protected]</a>>:<br></div><=
blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l=
eft:1px solid rgb(204,204,204);padding-left:1ex">On Sun, 07 Sep 2025, Thobi=
as Knudsen <<a href=3D"mailto:[email protected]" target=3D"_blank">thobk=
[email protected]</a>> wrote:<br>
>> It looks like you want runtime verification for the usage of the A=
PI.<br>
>> Did you know that URCU can now be compiled against ThreadSanitizer=
<br>
>> (TSAN)?=C2=A0 If a user misuses the API or makes incorrect assumpt=
ions about<br>
>> the guarantees offered by RCU, TSAN will most likely detect those<=
br>
>> issues.=C2=A0 Coupled with the other debug features we already hav=
e, this<br>
>> makes it very hard to not trigger an error path when the API is us=
ed<br>
>> incorrectly.<br>
><br>
> Really?! I've used TSAN and got a bunch of false positives, I beli=
eve, but<br>
> maybe they're not false positives? How do you remove the false pos=
itives,<br>
> or know that they're not false positives?<br>
<br>
Since 0.15.0, we've introduce an annotation layer (not part of the<br>
public API), making TSAN compatible with URCU.=C2=A0 See<br>
`include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD' macro.<br>
<br>
However, IIRC, you also need to compile URCU with the configuration<br>
option `--enable-compiler-atomic-builtins' so that atomic operations ar=
e<br>
implemented with the configured toolchain's builtin atomics.=C2=A0 I do=
n't<br>
recall if this was stricly necessary.<br>
<br>
If you encounter false positives with TSAN, please send me a minimal<br>
reproducible example together with:<br>
<br>
=C2=A0- the toolchain you=E2=80=99re using<br>
<br>
=C2=A0- the version of URCU<br>
<br>
=C2=A0- the configuration flags you used<br>
<br>
I will be happy to have a look.<br>
<br>
>> Note that certain kind of errors could actually be flag at compile=
time<br>
>> with the proper tooling.=C2=A0 For example, the Linux kernel uses =
a `__rcu'<br>
>> attribute that Sparse can understand to flag improper use of<br>
>> RCU=E2=80=91protected pointers.=C2=A0 I=E2=80=99d be very open to =
exposing something similar<br>
>> (an attribute) for static checkers.<br>
><br>
> wow thanks for the info! I knew compile time checks would be possible =
but<br>
> requiring compiler operability which is a higher hanging fruit for me.=
<br>
<br>
I don=E2=80=99t know the details of `__rcu' from the Linux kernel. I th=
ink it=E2=80=99s<br>
just a macro that expands to nothing by default, but Sparse treats it as<br=
>
an attribute.=C2=A0 I=E2=80=99m not sure exactly what checks Sparse perform=
s with it,<br>
but I suspect it involves traversing the program=E2=80=99s control-flow gra=
ph<br>
(CFG), ensuring that pointers marked with the `__rcu' qualifier are:<br=
>
<br>
=C2=A0 - obtained via rcu_dereference<br>
<br>
=C2=A0 - only dereferenced under RCU lock protection<br>
<br>
> Is '__rcu' compatible with custom concurrency? For example<br>
> rcu_dereference a pointer then locking a mutex inside the pointer then=
<br>
> unlock read then continue using the pointer?<br>
<br>
I don=E2=80=99t see why that would be a problem for the static-check algori=
thm I<br>
described above.=C2=A0 If a pointer needs to be protected by a mutex for<br=
>
mutation, that falls outside the scope of RCU, as far as I know.<br>
<br>
> I cant come up with something usefull other than a language rework. Is=
<br>
> it much work making the __urcu attribute?<br>
<br>
I suppose not.=C2=A0 On top of my head, it would involve adding some pointe=
r<br>
qualifier and function attributes to the primitives exposed by URCU.<br>
Users would also need to use the pointer qualifier when working with<br>
RCU-protected pointers.=C2=A0 The qualifier and the attributes would expand=
<br>
to nothing by default, letting static checkers defining them to internal<br=
>
values.=C2=A0 I suggest you read `Documentation/RCU/rcu_dereference.rst'=
; in<br>
the Linux kernel tree if you are interested.<br>
<br>
In its current state, this would not be very useful because none of the<br>
major compilers provide static analysis for RCU.=C2=A0 However, implementin=
g<br>
such analysis, as a plugin, wouldn=E2=80=99t be overly difficult for someon=
e<br>
familiar with Clang or GCC, I suppose.<br>
<br>
[...]<br>
<br>
Thanks,<br>
Olivier<br>
-- <br>
Olivier Dion<br>
EfficiOS Inc.<br>
<a href=3D"https://www.efficios.com" rel=3D"noreferrer" target=3D"_blank">h=
ttps://www.efficios.com</a><br>
</blockquote></div>
--0000000000000b9882063f2d7704--