Re: URCU feature request?
Thobias Knudsen via lttng-dev <[email protected]> Fri, 19 Sep 2025 22:42:05 +0200
| Newsgroups | org.lttng.lists.lttng-dev |
|---|---|
| Message-ID | <CAKGpcipB3Ctzg2XgfyQkEf6qbE+CURhFVUdN-PUvdJqBVbfdUA@mail.gmail.com> |
--000000000000718c09063f2d810d Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable I forgot to say Thanks, Thobias fre. 19. sep. 2025 kl. 22:39 skrev Thobias Knudsen <[email protected]>: > >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 fi= nd > a place where the thread sanitizer said there was a race and the write wa= s > 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 alg= orithm 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 outsid= e > 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 th= at > 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 abo= ut >> >> 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 >> positives, >> > 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 ti= me >> >> 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 >> but >> > 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 thi= nk 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 = with it, >> but I suspect it involves traversing the program=E2=80=99s control-flow = graph >> (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 alg= orithm 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 som= eone >> familiar with Clang or GCC, I suppose. >> >> [...] >> >> Thanks, >> Olivier >> -- >> Olivier Dion >> EfficiOS Inc. >> https://www.efficios.com >> > --000000000000718c09063f2d810d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">I forgot to say<div><br></div><div>Thanks,</div><div>Thobi= as</div></div><br><div class=3D"gmail_quote gmail_quote_container"><div dir= =3D"ltr" class=3D"gmail_attr">fre. 19. sep. 2025 kl. 22:39 skrev Thobias Kn= udsen <<a href=3D"mailto:[email protected]">[email protected]</a>>:<b= r></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex= ;border-left:1px solid rgb(204,204,204);padding-left:1ex"><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>>`inc= lude/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 operat= ions are<br>>implemented with the configured toolchain's builtin ato= mics.=C2=A0 I don't<br>>recall if this was strictly necessary.<br>&g= t;<br>>If you encounter false positives with TSAN, please send me a mini= mal<br>>reproducible example together with:<br>><br>> - the toolch= ain you=E2=80=99re using<br>><br>> - the version of URCU<br>><br>&= gt; - the configuration flags you used<br>><br>>I will be happy to ha= ve 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 switching to urcu v0.15.= 0 and setting the flags and macros for thread sanitizer=C2=A0support. Creat= ing 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 ar= ise=C2=A0because they are scattered all over tsm.c and test_tsm.c. If you h= ave linux debian family then you can clone my repo and run it:=C2=A0<a href= =3D"https://github.com/ThobiasKnudsen/Logos" target=3D"_blank">https://gith= ub.com/ThobiasKnudsen/Logos</a>. ./scripts/build.sh --debug --tsan &&am= p; ./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 ru= nning, the thread sanitizer produces warnings still. I did find a place whe= re the thread sanitizer said there=C2=A0was a race and the write was within= a node not yet inserted into the cds_lfht. That seems like a false positiv= e. 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.</div><d= iv><br></div><div>=C2=A0 Read of size 1 at 0x7b1400085ea1 by thread T6:<br>= =C2=A0 =C2=A0 #0 tsm_base_node_is_valid /Logos/src/tsm.c:721 (test_tsm+0xa5= 48)<br>=C2=A0 =C2=A0 #1 _tsm_tsm_type_is_valid /Logos/src/tsm.c:364 (test_t= sm+0xb1bf)<br>=C2=A0 =C2=A0 #2 tsm_node_is_valid /Logos/src/tsm.c:1553 (tes= t_tsm+0x9984)<br>=C2=A0 =C2=A0 #3 tsm_node_defer_free /Logos/src/tsm.c:1888= (test_tsm+0xb2fc)<br>=C2=A0 =C2=A0 #4 stress_thread /Logos/src/tests/test_= tsm.c:549 (test_tsm+0x46a6)<br><br>=C2=A0 Previous write of size 1 at 0x7b1= 400085ea1 by thread T9:<br>=C2=A0 =C2=A0 #0 tsm_base_node_create /Logos/src= /tsm.c:683 (test_tsm+0x6f2a)<br>=C2=A0 =C2=A0 #1 stress_thread /Logos/src/t= ests/test_tsm.c:843 (test_tsm+0x5ce0)<br></div><div><br></div><div>>I do= n=E2=80=99t see why that would be a problem for the static-check algorithm = I<br>>described above.=C2=A0 If a pointer needs to be protected by a mut= ex for<br>>mutation, that falls outside the scope of RCU, as far as I kn= ow.</div><div><br></div><div>If I understand correctly the __rcu checks tha= t reads are not done outside read sections and unsafe writes are not done a= t all. If this is correct then if you are using custom concurrency for __rc= u protected data outside the read section or unsafe writes is that allowed?= Because if it's not that would be a limitation for __rcu.=C2=A0</div><= /div><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">m= an. 8. sep. 2025 kl. 02:10 skrev Olivier Dion <<a href=3D"mailto:odion@e= fficios.com" target=3D"_blank">[email protected]</a>>:<br></div><blockq= uote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1p= x solid rgb(204,204,204);padding-left:1ex">On Sun, 07 Sep 2025, Thobias Knu= dsen <<a href=3D"mailto:[email protected]" target=3D"_blank">thobknu@gma= il.com</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> </blockquote></div> --000000000000718c09063f2d810d--