On Sat, May 27, 2023 at 9:18 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> It's also an example of something people need to be aware of: the
> auto-releasing is not ordered. That may or may not be a problem. It's
> not a problem for two identical locks, but it very much *can* be a
> problem if you mix different locks.
It is guaranteed. It would be nice to have it documented, but if you
look at the intermediate representation of this simple example:
#include <stdio.h>
int print_on_exit(void **f) {
if (*f) puts(*f);
*f = NULL;
}
int main(int argc) {
// prints "second" then "first"
void *__attribute__((__cleanup__(print_on_exit))) cleanup1 = "first";
void *__attribute__((__cleanup__(print_on_exit))) cleanup2 = "second";
}
... the implementation is introducing a scope and reusing the C++
try/finally implementation, and the "optimizations" that are allowed
when functions are not "throwing" anything. This is always the case
for C, so this (from f.c.005t.original, obtained with -c
-fdump-tree-all):
{
// the duplicated initializers are just an artifact of the AST
void * cleanup1 = (void *) "first";
void * cleanup2 = (void *) "second";
void * cleanup1 = (void *) "first";
try
{
void * cleanup2 = (void *) "second";
try
{
}
finally
{
print_on_exit (&cleanup2);
}
}
finally
{
print_on_exit (&cleanup1);
}
}
return 0;
becomes this as soon as exceptions are lowered (from f.c.013t.eh),
which is before any kind of optimization:
int main (int argc)
{
void * cleanup2;
void * cleanup1;
int D.3187;
cleanup1 = "first";
cleanup2 = "second";
print_on_exit (&cleanup2);
print_on_exit (&cleanup1);
cleanup1 = {CLOBBER(eol)};
cleanup2 = {CLOBBER(eol)};
D.3187 = 0;
goto <D.3188>;
<D.3188>:
return D.3187;
}
> So in the crazy above, the RCU lock may be released *after* the
> cgroup_mutex is released. Or before.
>
> I'm not convinced it's ordered even if you end up using explicit
> scopes, which you can obviously still do:
It is, see
void *__attribute__((__cleanup__(print_on_exit))) cleanup1 = "first";
{
void *__attribute__((__cleanup__(print_on_exit)))
cleanup2 = "second";
puts("begin nested scope");
}
puts("back to outer scope");
which prints
begin nested scope
second
back to outer scope
first
This is practically required by glibc's nasty
pthread_cleanup_push/pthread_cleanup_pop macros (the macros are nasty
even if you ignore glibc's implementation, but still):
# define pthread_cleanup_push(routine, arg) \
do { \
struct __pthread_cleanup_frame __clframe \
__attribute__ ((__cleanup__ (__pthread_cleanup_routine))) \
= { .__cancel_routine = (routine), .__cancel_arg = (arg), \
.__do_it = 1 };
/* Remove a cleanup handler installed by the matching pthread_cleanup_push.
If EXECUTE is non-zero, the handler function is called. */
# define pthread_cleanup_pop(execute) \
__clframe.__do_it = (execute); \
} while (0)
If the scope wasn't respected, pthread_cleanup_pop(1) would be broken
because pthread_cleanup_pop() must immediately execute the function if
its argument is nonzero.
> - I think the above is simpler and objectively better in every way
> from the explicitly scoped thing
It is simpler indeed, but a scope-based variant is useful too based on
my experience with QEMU. Maybe things like Python's with statement
have spoiled me, but I can't quite get used to the lone braces in
{
...
{
auto_release(mutex, &mutex);
...
}
...
}
So in QEMU we have both of them. In Linux that would be
auto_release(mutex, &mutex)
and
scoped(mutex, &mutex) {}
> - I do think that the auto-release can be very dangerous for locking,
> and people need to verify me about the ordering. Maybe the freeing
> order is well-defined.
It is but having it documented would be better.
> - I also suspect that to get maximum advantage of this all, we would
> have to get rid of -Wdeclaration-after-statement
Having both a declaration-based and a scope-based variant helps with
that too. You could add _Pragma("GCC diagnostic push/ignore/pop") to
the declaration-based macros but ouch... even without thinking of
whether clang supports it, which I didn't check.
Paolo
> That last point is something that some people will celebrate, but I do
> think it has helped keep our code base somewhat controlled, and made
> people think more about the variables they declare.
