All of lore.kernel.org
 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: 20+ 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 [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
2021-11-05 18:49 ` kernel test robot
2021-11-05 18:49   ` kernel test robot
2021-11-08 17:02 ` kernel test robot
2021-11-08 20:43 ` kernel test robot

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.