Mathieu Desnoyers <mathieu.desnoy...@efficios.com> wrote: > ----- On Aug 9, 2022, at 2:19 PM, Eric Wong via lttng-dev > lttng-dev@lists.lttng.org wrote: > > > Hello, I've noticed liburcu v0.11.4+ and later fails with the > > "mwrap" LD_PRELOAD malloc wrapper which initializes an rculfhash > > in a constructor. > > > > The problem happens when using _LGPL_SOURCE + rcu_dereference on > > a cds_lfht pointer: > > > > In file included from /tmp/b/include/urcu/pointer.h:39, > > from /tmp/b/include/urcu/urcu-bp.h:58, > > from /tmp/b/include/urcu-bp.h:2, > > from ../7ca7fe9c_regression.c:10: > > ../7ca7fe9c_regression.c: In function ‘main’: > > /tmp/b/include/urcu/static/pointer.h:101:18: error: invalid use of > > undefined > > type ‘struct cds_lfht’ > > 101 | __typeof__(p + 0) _________p1; \ > > | ^ > > /tmp/b/include/urcu/pointer.h:47:26: note: in expansion of macro > > ‘_rcu_dereference’ > > 47 | #define rcu_dereference _rcu_dereference > > | ^~~~~~~~~~~~~~~~ > > ../7ca7fe9c_regression.c:26:23: note: in expansion of macro > > ‘rcu_dereference’ > > 26 | struct cds_lfht *t = rcu_dereference(totals); > > | ^~~~~~~~~~~~~~~ > > > > Removing _LGPL_SOURCE avoids the problem, but I'd rather not > > introduce performance regressions. > > Fixed by commits: > > commit 2d466a6397dbc7af397d0fc10e327cc6cac76a5a > Author: Simon Marchi <simon.mar...@efficios.com> > Date: Wed Aug 17 11:24:25 2022 -0400 > > Fix: change method used by _rcu_dereference to strip type constness > > and > > commit fada682f0d6e680f18e3243aa0af607dfafcbd32 (review/master) > Author: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> > Date: Wed Aug 17 15:32:38 2022 -0400 > > Fix: add missing unused attribute to _rcu_dereference > > Those will be released in the 0.12 and 0.13 stable branches of liburcu.
Thanks. > Note that the 0.11 liburcu branch is end of life and will not have any > further release, so you may want to upgrade if you still use it. Unfortunately, enterprise distro users are likely to be stuck on 0.11 (or even older) for a long time :< > > > > In retrospect, my use of rcu_dereference seems unnecessary since > > constructor functions should always fire before any threads are > > created. > > > > That means I can safely replace the assignment w/ CMM_STORE_SHARED, > > and rcu_dereference with CMM_LOAD_SHARED, correct? > > I would not recommend it. Note that cds_lfht_new() starts a worker > thread (from a constructor) in your code. So there is concurrent > access after that hash table creation, even if you access it from > constructor code. Noted. However, I'm not exactly worried about losing reporting for a few allocations in the early boot process, it's inevitable. Allocations done by cds are the least of a Rubyist's problems. The original code didn't even bother using rcu_assign_pointer inside the resolve_malloc constructor :x https://80x24.org/mwrap-public/20220815212217.GA11237@dcvr/ postimage w/ patch applied: https://80x24.org/mwrap-public/477b1cb/s/?b=ext/mwrap/mwrap.c#n139 All the malloc wrappers themselves are immune to cds_lfht totals pointer being NULL. Thanks. _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
