Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Delyan Kratunov <delyank@fb.com>,
        "peterz@infradead.org" <peterz@infradead.org>
Cc:     "dietmar.eggemann@arm.com" <dietmar.eggemann@arm.com>,
        "bigeasy@linutronix.de" <bigeasy@linutronix.de>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "qais.yousef@arm.com" <qais.yousef@arm.com>,
        Delyan Kratunov <delyank@fb.com>,
        "andrii@kernel.org" <andrii@kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "vincent.guittot@linaro.org" <vincent.guittot@linaro.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "adharmap@quicinc.com" <adharmap@quicinc.com>,
        "alexei.starovoitov@gmail.com" <alexei.starovoitov@gmail.com>,
        "andrii.nakryiko@gmail.com" <andrii.nakryiko@gmail.com>,
        "ast@kernel.org" <ast@kernel.org>,
        "Kenta.Tada@sony.com" <Kenta.Tada@sony.com>,
        "rdunlap@infradead.org" <rdunlap@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bristot@redhat.com" <bristot@redhat.com>,
        "ebiederm@xmission.com" <ebiederm@xmission.com>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "ed.tsai@mediatek.com" <ed.tsai@mediatek.com>,
        "legion@kernel.org" <legion@kernel.org>,
        "acme@kernel.org" <acme@kernel.org>,
        "u.kleine-koenig@pengutronix.de" <u.kleine-koenig@pengutronix.de>,
        "valentin.schneider@arm.com" <valentin.schneider@arm.com>,
        "juri.lelli@redhat.com" <juri.lelli@redhat.com>,
        "namhyung@kernel.org" <namhyung@kernel.org>
Subject: Re: [PATCH v2] sched/tracing: append prev_state to tp args instead
In-Reply-To: <c8a6930dfdd58a4a5755fc01732675472979732b.camel@fb.com>
References: <20220510082940.GA100765@worktop.programming.kicks-ass.net>
 <c8a6930dfdd58a4a5755fc01732675472979732b.camel@fb.com>
Date:   Thu, 12 May 2022 01:40:57 +0200
Message-ID: <8735hfy6wm.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, May 11 2022 at 18:28, Delyan Kratunov wrote:
> Commit fa2c3254d7cf (sched/tracing: Don't re-read p->state when emitting
> sched_switch event, 2022-01-20) added a new prev_state argument to the
> sched_switch tracepoint, before the prev task_struct pointer.
>
> This reordering of arguments broke BPF programs that use the raw
> tracepoint (e.g. tp_btf programs). The type of the second argument has
> changed and existing programs that assume a task_struct* argument
> (e.g. for bpf_task_storage access) will now fail to verify.
>
> If we instead append the new argument to the end, all existing programs
> would continue to work and can conditionally extract the prev_state
> argument on supported kernel versions.

If we instead? ... would continue to work?

  https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
  https://www.kernel.org/doc/html/latest/process/submitting-patches.html#submittingpatches

It's not too hard to actually follow documented rules. Something like
this:

  "Undo the reordering and move the new argument last, which ensures
   that all existing programs continue to work."

Hmm?

What's worse is that the changelog is missing a clear statement that
this is a one-time change which cannot be abused to turn tracepoints
into unmodifiable ABI as you stated in:

 https://lore.kernel.org/lkml/CAEf4BzaEo+dxSRJZHQiXYrj-a3_B-eODZUxGh3HrnPjquMYFXQ@mail.gmail.com

Thanks,

        tglx