[lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Martin Wilck via lttng-dev]

In multipath-tools, we are using a custom RCU helper thread, which is cleaned out
on exit:

https://github.com/opensvc/multipath-tools/blob/23a01fa679481ff1144139222fbd2c4c863b78f8/multipathd/main.c#L3058

I put a call to rcu_barrier() there in order to make sure all callbacks had finished
before detaching the helper thread.

Now we got a report that rcu_barrier() isn't available before user-space RCU 0.8 
(https://github.com/opensvc/multipath-tools/issues/5) (and RHEL7 / Centos7 
still has 0.7.16).

Question: was it over-cautious or otherwise wrong to call rcu_barrier() before
set_thread_call_rcu_data(NULL)? Can we maybe just skip this call? If no, what
would be the recommended way for liburcu < 0.8 to dissociate a helper thread?

(Note: I'm not currently subscribed to lttng-dev).

Regards and thanks,
Martin



_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Mathieu Desnoyers via lttng-dev]

----- On Apr 29, 2021, at 9:49 AM, lttng-dev lttng-dev@lists.lttng.org wrote:

> In multipath-tools, we are using a custom RCU helper thread, which is cleaned
> out
> on exit:
> 
> https://github.com/opensvc/multipath-tools/blob/23a01fa679481ff1144139222fbd2c4c863b78f8/multipathd/main.c#L3058
> 
> I put a call to rcu_barrier() there in order to make sure all callbacks had
> finished
> before detaching the helper thread.
> 
> Now we got a report that rcu_barrier() isn't available before user-space RCU 0.8
> (https://github.com/opensvc/multipath-tools/issues/5) (and RHEL7 / Centos7
> still has 0.7.16).
> 
> Question: was it over-cautious or otherwise wrong to call rcu_barrier() before
> set_thread_call_rcu_data(NULL)? Can we maybe just skip this call? If no, what
> would be the recommended way for liburcu < 0.8 to dissociate a helper thread?
> 
> (Note: I'm not currently subscribed to lttng-dev).

First of all, there is a significant reason why liburcu does not free the "default"
call_rcu worker thread data structures at process exit. This is caused by the fact that
a call_rcu callback may very well invoke call_rcu() to re-enqueue more work.

AFAIU this is somewhat similar to what happens to the Linux kernel RCU implementation
when the machine needs to be shutdown or rebooted: there may indeed never be any point
in time where it is safe to free the call_rcu worker thread data structures without leaks,
due to the fact that a call_rcu callback may re-enqueue further work indefinitely.

So my understanding is that you implement your own call rcu worker thread because the
one provided by liburcu leaks data structure on process exit, and you expect that
call rcu_barrier once will suffice to ensure quiescence of the call rcu worker thread
data structures. Unfortunately, this does not cover the scenario where a call_rcu
callback re-enqueues additional work.

So without knowing more details on the reasons why you wish to clean up memory at
process exit, and why it would be valid to do so in your particular use-case, it's
rather difficult for me to elaborate a complete answer.

I can see that maybe we could change liburcu to make it so that we free all
call_rcu data structures _if_ they happen to be empty of callbacks at process exit,
after invoking one rcu_barrier. That should take care of not leaking data structures
in the common case where call_rcu does not enqueue further callbacks.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Martin Wilck via lttng-dev]

On Fri, 2021-04-30 at 14:41 -0400, Mathieu Desnoyers wrote:
> ----- On Apr 29, 2021, at 9:49 AM, lttng-dev 
> lttng-dev@lists.lttng.org wrote:
> 
> > In multipath-tools, we are using a custom RCU helper thread, which
> > is cleaned
> > out
> > on exit:
> > 
> > https://github.com/opensvc/multipath-tools/blob/23a01fa679481ff1144139222fbd2c4c863b78f8/multipathd/main.c#L3058
> > 
> > I put a call to rcu_barrier() there in order to make sure all
> > callbacks had
> > finished
> > before detaching the helper thread.
> > 
> > Now we got a report that rcu_barrier() isn't available before user-
> > space RCU 0.8
> > (https://github.com/opensvc/multipath-tools/issues/5) (and RHEL7 /
> > Centos7
> > still has 0.7.16).
> > 
> > Question: was it over-cautious or otherwise wrong to call
> > rcu_barrier() before
> > set_thread_call_rcu_data(NULL)? Can we maybe just skip this call?
> > If no, what
> > would be the recommended way for liburcu < 0.8 to dissociate a
> > helper thread?
> > 
> > (Note: I'm not currently subscribed to lttng-dev).
> 
> First of all, there is a significant reason why liburcu does not free
> the "default"
> call_rcu worker thread data structures at process exit. This is
> caused by the fact that
> a call_rcu callback may very well invoke call_rcu() to re-enqueue
> more work.
> 
> AFAIU this is somewhat similar to what happens to the Linux kernel
> RCU implementation
> when the machine needs to be shutdown or rebooted: there may indeed
> never be any point
> in time where it is safe to free the call_rcu worker thread data
> structures without leaks,
> due to the fact that a call_rcu callback may re-enqueue further work
> indefinitely.
> 
> So my understanding is that you implement your own call rcu worker
> thread because the
> one provided by liburcu leaks data structure on process exit, and you
> expect that
> call rcu_barrier once will suffice to ensure quiescence of the call
> rcu worker thread
> data structures. Unfortunately, this does not cover the scenario
> where a call_rcu
> callback re-enqueues additional work.

I understand. In multipath-tools, we only have one callback, which
doesn't re-enqueue any work. Our callback really just calls free() on a
data structure. And it's unlikely that we'll get more RCU callbacks any
time soon.

So, to clarify my question: Does it make sense to call rcu_barrier()
before set_thread_call_rcu_data(NULL) in this case? If yes, is there an
alternative for safely detaching the custom RCU thread if rcu_barrier()
is unavailable?

> So without knowing more details on the reasons why you wish to clean
> up memory at
> process exit, and why it would be valid to do so in your particular
> use-case, it's
> rather difficult for me to elaborate a complete answer.

multipathd is a long-running process, so being wary of memory leaks is
important. valgrind tests pop up an ugly warning about liburcu - it's
obviously not a big issue, as it occurs only on exit, but it makes a
negative impression on users running memory leak tests. It's possible
to work around that by using valgrind "suppressions", but so far my
policy was to use these only as last resort measure, in case we
couldn't find any way to work around it in our code. That's why I came
up with the "custom RCU thread" approach.

Anyway, from what you're saying, it might be be better to simply accept
the fact that this pseudo-memory-leak exists than trying to fix it in
an unsafe way with older liburcu versions.

> I can see that maybe we could change liburcu to make it so that we
> free all
> call_rcu data structures _if_ they happen to be empty of callbacks at
> process exit,
> after invoking one rcu_barrier. That should take care of not leaking
> data structures
> in the common case where call_rcu does not enqueue further callbacks.
> 
> Thoughts ?

That would be nice, but it wouldn't help me in the specific case, where
I have to deal with an old version of liburcu.

Perhaps you could also consider an API extension by which an
application could tell liburcu that it's exiting, and no further
callbacks should be scheduled?

Thanks,
Martin


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Mathieu Desnoyers via lttng-dev]

----- On May 5, 2021, at 3:54 AM, Martin Wilck mwilck@suse.com wrote:

> On Fri, 2021-04-30 at 14:41 -0400, Mathieu Desnoyers wrote:
>> ----- On Apr 29, 2021, at 9:49 AM, lttng-dev
>> lttng-dev@lists.lttng.org wrote:
>> 
>> > In multipath-tools, we are using a custom RCU helper thread, which
>> > is cleaned
>> > out
>> > on exit:
>> > 
>> > https://github.com/opensvc/multipath-tools/blob/23a01fa679481ff1144139222fbd2c4c863b78f8/multipathd/main.c#L3058
>> > 
>> > I put a call to rcu_barrier() there in order to make sure all
>> > callbacks had
>> > finished
>> > before detaching the helper thread.
>> > 
>> > Now we got a report that rcu_barrier() isn't available before user-
>> > space RCU 0.8
>> > (https://github.com/opensvc/multipath-tools/issues/5) (and RHEL7 /
>> > Centos7
>> > still has 0.7.16).
>> > 
>> > Question: was it over-cautious or otherwise wrong to call
>> > rcu_barrier() before
>> > set_thread_call_rcu_data(NULL)? Can we maybe just skip this call?
>> > If no, what
>> > would be the recommended way for liburcu < 0.8 to dissociate a
>> > helper thread?
>> > 
>> > (Note: I'm not currently subscribed to lttng-dev).
>> 
>> First of all, there is a significant reason why liburcu does not free
>> the "default"
>> call_rcu worker thread data structures at process exit. This is
>> caused by the fact that
>> a call_rcu callback may very well invoke call_rcu() to re-enqueue
>> more work.
>> 
>> AFAIU this is somewhat similar to what happens to the Linux kernel
>> RCU implementation
>> when the machine needs to be shutdown or rebooted: there may indeed
>> never be any point
>> in time where it is safe to free the call_rcu worker thread data
>> structures without leaks,
>> due to the fact that a call_rcu callback may re-enqueue further work
>> indefinitely.
>> 
>> So my understanding is that you implement your own call rcu worker
>> thread because the
>> one provided by liburcu leaks data structure on process exit, and you
>> expect that
>> call rcu_barrier once will suffice to ensure quiescence of the call
>> rcu worker thread
>> data structures. Unfortunately, this does not cover the scenario
>> where a call_rcu
>> callback re-enqueues additional work.
> 
> I understand. In multipath-tools, we only have one callback, which
> doesn't re-enqueue any work. Our callback really just calls free() on a
> data structure. And it's unlikely that we'll get more RCU callbacks any
> time soon.
> 
> So, to clarify my question: Does it make sense to call rcu_barrier()
> before set_thread_call_rcu_data(NULL) in this case?

Yes, it would ensure that all pending callbacks are executed prior to
removing the worker thread. And considering that you don't have chained
callbacks, it makes sense to invoke rcu_barrier() only once.

> If yes, is there an
> alternative for safely detaching the custom RCU thread if rcu_barrier()
> is unavailable?

I suspect you could re-implement something similar to rcu_barrier() within
your application through call_rcu and a rendez-vous synchronization. It
all depends on how much complexity you want to add to your application
for the sake of not leaking data structures when using old versions of
liburcu.

> 
>> So without knowing more details on the reasons why you wish to clean
>> up memory at
>> process exit, and why it would be valid to do so in your particular
>> use-case, it's
>> rather difficult for me to elaborate a complete answer.
> 
> multipathd is a long-running process, so being wary of memory leaks is
> important. valgrind tests pop up an ugly warning about liburcu - it's
> obviously not a big issue, as it occurs only on exit, but it makes a
> negative impression on users running memory leak tests. It's possible
> to work around that by using valgrind "suppressions", but so far my
> policy was to use these only as last resort measure, in case we
> couldn't find any way to work around it in our code. That's why I came
> up with the "custom RCU thread" approach.
> 
> Anyway, from what you're saying, it might be be better to simply accept
> the fact that this pseudo-memory-leak exists than trying to fix it in
> an unsafe way with older liburcu versions.

If we push this line of thinking to the extreme, we should look into what
improvement should be to to liburcu upstream so we fix this situation in
the future, and then you can decide how you want to handle legacy liburcu
on your side.

>> I can see that maybe we could change liburcu to make it so that we
>> free all
>> call_rcu data structures _if_ they happen to be empty of callbacks at
>> process exit,
>> after invoking one rcu_barrier. That should take care of not leaking
>> data structures
>> in the common case where call_rcu does not enqueue further callbacks.
>> 
>> Thoughts ?
> 
> That would be nice, but it wouldn't help me in the specific case, where
> I have to deal with an old version of liburcu.
> 
> Perhaps you could also consider an API extension by which an
> application could tell liburcu that it's exiting, and no further
> callbacks should be scheduled?

But then how is the application supposed to deal with this ? For instance,
the call_rcu callback could be used to implement a condition variable rendez-vous
point which blocks other parts of the application until it is executed.

I have a few ideas on how to deal with this in liburcu upstream:

1) We could implement library destructor functions which cleanup the call rcu
   worker threads (and their data structures) _only if_ they are quiescent and
   their associated callback list is empty.

2.1) We could document that the application needs to invoke rcu_barrier() before
     it exits if it wishes to ensure that all call_rcu callbacks are executed before
     it exits. We should document that if the application chains call_rcu callbacks,
     it needs to invoke rcu_barrier() as many times as there are consecutive chaining.
     And of course, that a never-ending chaining of call_rcu callbacks will necessarily
     lead to memory leaks at application exit.

2.2) Alternatively, we could have the rcu_barrier invoked from within liburcu's destructor.
     The number of times rcu_barrier would be invoked could be configured through a new API.
     The default could be that rcu_barrier is invoked once. An application could choose to
     override this so rcu_barrier is never called at application exit if it cares more about
     exiting quickly than leaking memory.

I would slightly favor approaches (1) + (2.1), because it leaves all flexibility to the
application: if call_rcu is invoked from within a library, then that library is free to
choose how many times it needs to invoke rcu_barrier in its own library destructor (e.g.
on dlclose()).

In order to make sure the "common use-case" does not leak memory though, we could make sure
liburcu does one rcu_barrier and conditionally cleanup the worker thread + data structures if
the callback list is empty after the barrier.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Paul E. McKenney via lttng-dev]

On Wed, May 05, 2021 at 10:46:58AM -0400, Mathieu Desnoyers wrote:
> ----- On May 5, 2021, at 3:54 AM, Martin Wilck mwilck@suse.com wrote:
> 
> > On Fri, 2021-04-30 at 14:41 -0400, Mathieu Desnoyers wrote:
> >> ----- On Apr 29, 2021, at 9:49 AM, lttng-dev
> >> lttng-dev@lists.lttng.org wrote:
> >> 
> >> > In multipath-tools, we are using a custom RCU helper thread, which
> >> > is cleaned
> >> > out
> >> > on exit:
> >> > 
> >> > https://github.com/opensvc/multipath-tools/blob/23a01fa679481ff1144139222fbd2c4c863b78f8/multipathd/main.c#L3058
> >> > 
> >> > I put a call to rcu_barrier() there in order to make sure all
> >> > callbacks had
> >> > finished
> >> > before detaching the helper thread.
> >> > 
> >> > Now we got a report that rcu_barrier() isn't available before user-
> >> > space RCU 0.8
> >> > (https://github.com/opensvc/multipath-tools/issues/5) (and RHEL7 /
> >> > Centos7
> >> > still has 0.7.16).
> >> > 
> >> > Question: was it over-cautious or otherwise wrong to call
> >> > rcu_barrier() before
> >> > set_thread_call_rcu_data(NULL)? Can we maybe just skip this call?
> >> > If no, what
> >> > would be the recommended way for liburcu < 0.8 to dissociate a
> >> > helper thread?
> >> > 
> >> > (Note: I'm not currently subscribed to lttng-dev).
> >> 
> >> First of all, there is a significant reason why liburcu does not free
> >> the "default"
> >> call_rcu worker thread data structures at process exit. This is
> >> caused by the fact that
> >> a call_rcu callback may very well invoke call_rcu() to re-enqueue
> >> more work.
> >> 
> >> AFAIU this is somewhat similar to what happens to the Linux kernel
> >> RCU implementation
> >> when the machine needs to be shutdown or rebooted: there may indeed
> >> never be any point
> >> in time where it is safe to free the call_rcu worker thread data
> >> structures without leaks,
> >> due to the fact that a call_rcu callback may re-enqueue further work
> >> indefinitely.
> >> 
> >> So my understanding is that you implement your own call rcu worker
> >> thread because the
> >> one provided by liburcu leaks data structure on process exit, and you
> >> expect that
> >> call rcu_barrier once will suffice to ensure quiescence of the call
> >> rcu worker thread
> >> data structures. Unfortunately, this does not cover the scenario
> >> where a call_rcu
> >> callback re-enqueues additional work.
> > 
> > I understand. In multipath-tools, we only have one callback, which
> > doesn't re-enqueue any work. Our callback really just calls free() on a
> > data structure. And it's unlikely that we'll get more RCU callbacks any
> > time soon.
> > 
> > So, to clarify my question: Does it make sense to call rcu_barrier()
> > before set_thread_call_rcu_data(NULL) in this case?
> 
> Yes, it would ensure that all pending callbacks are executed prior to
> removing the worker thread. And considering that you don't have chained
> callbacks, it makes sense to invoke rcu_barrier() only once.

If you do have chained callbacks, one trick is to:

1.	Prevent your application from doing any more new invocations
	of call_rcu().

2.	Set a flag that prevents any future callbacks from chaining.

3.	Do two calls to rcu_barrier(), one to wait for pre-existing
	callbacks and another to wait for any additional chained
	callbacks that happened concurrently with #2 above.

							Thanx, Paul
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] User-space RCU: call rcu_barrier() before dissociating helper thread?

[Martin Wilck via lttng-dev]

Hello Mathieu,

thanks again.

On Wed, 2021-05-05 at 10:46 -0400, Mathieu Desnoyers wrote:
> > > 
> > > So my understanding is that you implement your own call rcu
> > > worker
> > > thread because the
> > > one provided by liburcu leaks data structure on process exit, and
> > > you
> > > expect that
> > > call rcu_barrier once will suffice to ensure quiescence of the
> > > call
> > > rcu worker thread
> > > data structures. Unfortunately, this does not cover the scenario
> > > where a call_rcu
> > > callback re-enqueues additional work.
> > 
> > I understand. In multipath-tools, we only have one callback, which
> > doesn't re-enqueue any work. Our callback really just calls free()
> > on a
> > data structure. And it's unlikely that we'll get more RCU callbacks
> > any
> > time soon.
> > 
> > So, to clarify my question: Does it make sense to call
> > rcu_barrier()
> > before set_thread_call_rcu_data(NULL) in this case?
> 
> Yes, it would ensure that all pending callbacks are executed prior to
> removing the worker thread. And considering that you don't have
> chained
> callbacks, it makes sense to invoke rcu_barrier() only once.
> 
> > If yes, is there an
> > alternative for safely detaching the custom RCU thread if
> > rcu_barrier()
> > is unavailable?
> 
> I suspect you could re-implement something similar to rcu_barrier()
> within
> your application through call_rcu and a rendez-vous synchronization.
> It
> all depends on how much complexity you want to add to your
> application
> for the sake of not leaking data structures when using old versions
> of
> liburcu.

Thanks, but I'm definitely not up to that task :-) As I said, our
software is using liburcu in a rather trivial way. What I'll do is
avoid creating and tearing down the custom RCU thread with liburcu
older than 0.8. Thus we'll end up with a minor memory leak on the
affected (old) distributions, which is acceptable.

> > 
> > > So without knowing more details on the reasons why you wish to
> > > clean
> > > up memory at
> > > process exit, and why it would be valid to do so in your
> > > particular
> > > use-case, it's
> > > rather difficult for me to elaborate a complete answer.
> > 
> > multipathd is a long-running process, so being wary of memory leaks
> > is
> > important. valgrind tests pop up an ugly warning about liburcu -
> > it's
> > obviously not a big issue, as it occurs only on exit, but it makes
> > a
> > negative impression on users running memory leak tests. It's
> > possible
> > to work around that by using valgrind "suppressions", but so far my
> > policy was to use these only as last resort measure, in case we
> > couldn't find any way to work around it in our code. That's why I
> > came
> > up with the "custom RCU thread" approach.
> > 
> > Anyway, from what you're saying, it might be be better to simply
> > accept
> > the fact that this pseudo-memory-leak exists than trying to fix it
> > in
> > an unsafe way with older liburcu versions.
> 
> If we push this line of thinking to the extreme, we should look into
> what
> improvement should be to to liburcu upstream so we fix this situation
> in
> the future, and then you can decide how you want to handle legacy
> liburcu
> on your side.
> 
> > > I can see that maybe we could change liburcu to make it so that
> > > we
> > > free all
> > > call_rcu data structures _if_ they happen to be empty of
> > > callbacks at
> > > process exit,
> > > after invoking one rcu_barrier. That should take care of not
> > > leaking
> > > data structures
> > > in the common case where call_rcu does not enqueue further
> > > callbacks.
> > > 
> > > Thoughts ?
> > 
> > That would be nice, but it wouldn't help me in the specific case,
> > where
> > I have to deal with an old version of liburcu.
> > 
> > Perhaps you could also consider an API extension by which an
> > application could tell liburcu that it's exiting, and no further
> > callbacks should be scheduled?
> 
> But then how is the application supposed to deal with this ? For
> instance,
> the call_rcu callback could be used to implement a condition variable
> rendez-vous
> point which blocks other parts of the application until it is
> executed.
> 
> I have a few ideas on how to deal with this in liburcu upstream:
> 
> 1) We could implement library destructor functions which cleanup the
> call rcu
>    worker threads (and their data structures) _only if_ they are
> quiescent and
>    their associated callback list is empty.

My idea would be to use an atomic variable that's checked before
every invocation of call_rcu(), in the entire application. If I
understood Paul's response correctly, this combined with two
invocations of rcu_barrier() should do the trick.

The documentation could include an example how to do this correctly.

> 
> 2.1) We could document that the application needs to invoke
> rcu_barrier() before
>      it exits if it wishes to ensure that all call_rcu callbacks are
> executed before
>      it exits. We should document that if the application chains
> call_rcu callbacks,
>      it needs to invoke rcu_barrier() as many times as there are
> consecutive chaining.
>      And of course, that a never-ending chaining of call_rcu
> callbacks will necessarily
>      lead to memory leaks at application exit.

This sounds perfectly reasonable. 

> 
> 2.2) Alternatively, we could have the rcu_barrier invoked from within
> liburcu's destructor.
>      The number of times rcu_barrier would be invoked could be
> configured through a new API.
>      The default could be that rcu_barrier is invoked once. An
> application could choose to
>      override this so rcu_barrier is never called at application exit
> if it cares more about
>      exiting quickly than leaking memory.
> 
> I would slightly favor approaches (1) + (2.1), because it leaves all
> flexibility to the
> application: if call_rcu is invoked from within a library, then that
> library is free to
> choose how many times it needs to invoke rcu_barrier in its own
> library destructor (e.g.
> on dlclose()).

Ack. If RCU is used by an application _and_ one or more libraries the
application uses, my global flag obviously doesn't work. I believe a
library using RCU should provide some means to signal exiting state via
its API, to avoid further callbacks/chaining in the library.


> In order to make sure the "common use-case" does not leak memory
> though, we could make sure
> liburcu does one rcu_barrier and conditionally cleanup the worker
> thread + data structures if
> the callback list is empty after the barrier.

Not sure about that. It would be a semantic change in the library, no?
I suppose calling rcu_barrier() one more time doesn't hurt, but it's
not my place to advise you about that. Apparently, other users of
liburcu so far haven't complained about the leak, so you might as well
just add a note to the documentation. For a simple use case such as
multipathd, the trick with the custom RCU thread seems to be a valid
workaround. That's fine for me.

Thanks a lot,
Martin


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev