mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Gustavo A. R. Silva" <gustavoars@kernel.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: Tue, 5 Oct 2021 20:09:35 -0400	[thread overview]
Message-ID: <20211005200935.2429ec2c@rorschach.local.home> (raw)
In-Reply-To: <20211005193557.GA881195@embeddedor>

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.

-- Steve


> 
> Thanks!
> --
> Gustavo
> 
> [1] https://lore.kernel.org/all/CAG48ez2pOns4vF9M_4ubMJ+p9YFY29udMaH0wm8UuCwGQ4ZZAQ@mail.gmail.com/
> [2] https://lore.kernel.org/all/CAG48ez04Fj=1p61KAxAQWZ3f_z073fVUr8LsQgtKA9c-kcHmDQ@mail.gmail.com/#t
> [3] https://lore.kernel.org/all/CAG48ez1LoTLmHnAKFZCQFSvcb13Em6kc8y1xO8sNwyvzB=D2Lg@mail.gmail.com/


  reply	other threads:[~2021-10-06  0:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05  5:39 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 [this message]
2021-10-06 21:14               ` Gustavo A. R. Silva
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=20211005200935.2429ec2c@rorschach.local.home \
    --to=rostedt@goodmis.org \
    --cc=bristot@kernel.org \
    --cc=gustavoars@kernel.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    /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

Powered by JetHome