Re: [PATCH v3 56/57] perf: Simplify perf_pmu_output_stop()

[Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>]

This patch looks completely broken to me.

You now do

                if (err == -EAGAIN)
                        goto restart;

*within* the RCU-guarded section, and the "goto restart" will guard it again.

So no. Sending out a series of 57 patches that can have these kinds of
bugs in it is not ok. By patch 56 (which just happened to come in
fairly early for me), all sane peoples eyes have glazed over and they
don't react to this kind of breakage any more.

                Linus

On Mon, Jun 12, 2023 at 2:39 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7977,7 +7977,8 @@ static void perf_pmu_output_stop(struct
>         int err, cpu;
>
>  restart:
> -       rcu_read_lock();
> +       /* cannot have a label in front of a decl */;
> +       guard(rcu)();
>         list_for_each_entry_rcu(iter, &event->rb->event_list, rb_entry) {
>                 /*
>                  * For per-CPU events, we need to make sure that neither they
> @@ -7993,12 +7994,9 @@ static void perf_pmu_output_stop(struct
>                         continue;
>
>                 err = cpu_function_call(cpu, __perf_pmu_output_stop, event);
> -               if (err == -EAGAIN) {
> -                       rcu_read_unlock();
> +               if (err == -EAGAIN)
>                         goto restart;
> -               }
>         }
> -       rcu_read_unlock();
>  }