MIME-Version: 1.0
References: <20230201163420.1579014-1-revest@chromium.org> <20230201163420.1579014-2-revest@chromium.org>
 <Y9vPAdFBJF/gKXaO@FVFF77S0Q05N.cambridge.arm.com> <CABRcYmLrYXuP-yio0dy4WskENn81Qw2WS0ArMp=rdHuiGyjYhQ@mail.gmail.com>
In-Reply-To: <CABRcYmLrYXuP-yio0dy4WskENn81Qw2WS0ArMp=rdHuiGyjYhQ@mail.gmail.com>
From:   Florent Revest <revest@chromium.org>
Date:   Tue, 7 Feb 2023 16:21:49 +0100
Message-ID: <CABRcYmL7exDGnBTzj-DHLSf49_tz6bSDfjn9CDLO9ZEbfFyh-w@mail.gmail.com>
Subject: Re: [PATCH 1/8] ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org,
        catalin.marinas@arm.com, will@kernel.org, rostedt@goodmis.org,
        mhiramat@kernel.org, ast@kernel.org, daniel@iogearbox.net,
        andrii@kernel.org, kpsingh@kernel.org, jolsa@kernel.org,
        xukuohai@huaweicloud.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 2, 2023 at 6:37 PM Florent Revest <revest@chromium.org> wrote:
>
> On Thu, Feb 2, 2023 at 4:02 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > Looking at samples/ftrace/, as of this patch we have a few samples that are
> > almost identical, modulo the function being traced, and some different register
> > shuffling for arguments:
> >
> > * ftrace-direct.c and ftrace-direct-multi.c
> > * ftrace-direct-modify.c and ftrace-direct-modify
> >
> > ... perhaps it would be better to just delete the !multi versions ?
>
> The multi versions hook two functions and the !multi hook just one but
> I agree that this granularity in coverage is probably just a
> maintenance burden and doesn't help with much! :)
> I'll delete the !multi in v2, as part of the patch 2 and patch 1 will
> just migrate the selftest to use the multi API.

Actually, I'm not sure anymore if we should delete the !multi samples...

I realized that they are also used as part of the ftrace selftests in:
- tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
- tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc

It does not really make sense to use the ftrace-direct-muti sample as
a drop-in replacement for the ftrace-direct sample there since they
don't really do the same thing so we would either need to change the
test a bit or the multi sample.
Also, we would still need to adapt the ftrace-direct-too sample since
it has no multi equivalent and is required there.

It's certainly doable but now it feels to me like going one step too
far with the refactoring within the scope of this series. Do you think
it's worth it ? I have 70% of that work done already so I'm happy to
finish it but I thought I should check back before sending a v2 that's
more complex than anticipated.