Mathieu Desnoyers 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
> 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
> 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
