From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>,
Daniel Bristot de Oliveira <bristot@kernel.org>,
linux-kernel@vger.kernel.org, Jann Horn <jannh@google.com>
Subject: Re: [PATCH][next] ftrace: Fix -Wcast-function-type warnings on powerpc64
Date: Wed, 6 Oct 2021 16:14:26 -0500 [thread overview]
Message-ID: <20211006211426.GA916113@embeddedor> (raw)
In-Reply-To: <20211005200935.2429ec2c@rorschach.local.home>
On Tue, Oct 05, 2021 at 08:09:35PM -0400, Steven Rostedt wrote:
> On Tue, 5 Oct 2021 14:35:57 -0500
> "Gustavo A. R. Silva" <gustavoars@kernel.org> wrote:
>
> > On Tue, Oct 05, 2021 at 03:08:07PM -0400, Steven Rostedt wrote:
> > [..]
> > > Or did you not remove your patch first?
> >
> > Yep; that was the problem.
> >
> > I now applied it to a clean tree and the warnings went away.
> >
> > However, I'm a bit concerned about the following Jann's comments:
>
> I should have replied back then, but I'll do that now (and added Jann
> to the CC)
>
> >
> > "the real issue here is that ftrace_func_t is defined as a fixed
> > type, but actually has different types depending on the architecture?
> > If so, it might be cleaner to define ftrace_func_t differently
> > depending on architecture, or something like that?"[1]
>
> It's not dependent on the architecture. It's dependent on what the
> architecture has implemented. There's nothing limiting the arch to use
> the normal method, except that nobody implemented the updates.
>
> As I changed the core API, it affected the architectures, and since I
> don't know how to update all the architectures that use that API, and
> do not have the hardware to test it, I made it so architectures can
> slowly be updated when their maintainers get time to. This was years
> ago, and not much has been done.
>
> >
> > "Would it not be possible to have two function types (#define'd as the
> > same if ARCH_SUPPORTS_FTRACE_OPS), and then ensure that ftrace_func_t
> > is only used as ftrace_asm_func_t if ARCH_SUPPORTS_FTRACE_OPS?"[2]
> >
> > "Essentially my idea here is to take the high-level rule "you can only
> > directly call ftrace_func_t-typed functions from assembly if
> > ARCH_SUPPORTS_FTRACE_OPS", and encode it in the type system. And then
> > the compiler won't complain as long as we make sure that we never cast
> > between the two types under ARCH_SUPPORTS_FTRACE_OPS==0."[3]
> >
> > So, is this linker approach really a good solution to this problem? :)
> >
> > What's the main problem with what Jann suggests?
>
> The main issue is I want no more #ifdef's in the main code. There's too
> many already and it makes it difficult to maintain. I want to get rid
> of them, not add more. So anything that adds more #ifdef's to the main
> code, I will NACK.
>
> Which I guess leaves us with either the linker trick, or having all
> the archs get updated to support the latest ftrace features, and we can
> remove the current #ifdefs.
OK. Are you going to apply your patch any time soon? So, I can go and
enable -Wcast-function-type in my -next tree. :)
Thanks
--
Gustavo
next prev parent reply other threads:[~2021-10-06 21:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-05 5:39 [PATCH][next] ftrace: Fix -Wcast-function-type warnings on powerpc64 Gustavo A. R. Silva
2021-10-05 15:17 ` Steven Rostedt
2021-10-05 16:18 ` Gustavo A. R. Silva
2021-10-05 16:35 ` Steven Rostedt
2021-10-05 16:50 ` Gustavo A. R. Silva
2021-10-05 16:51 ` Steven Rostedt
2021-10-05 19:08 ` Steven Rostedt
2021-10-05 19:35 ` Gustavo A. R. Silva
2021-10-06 0:09 ` Steven Rostedt
2021-10-06 21:14 ` Gustavo A. R. Silva [this message]
2021-10-06 21:14 ` Steven Rostedt
2021-10-06 21:23 ` Gustavo A. R. Silva
2021-10-13 1:40 ` Gustavo A. R. Silva
2021-10-13 2:23 ` Steven Rostedt
2021-10-13 2:39 ` Gustavo A. R. Silva
2021-10-13 2:44 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211006211426.GA916113@embeddedor \
--to=gustavoars@kernel.org \
--cc=bristot@kernel.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox