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;
MIME-Version: 1.0
References: <20220816032846.2579217-1-imagedong@tencent.com>
 <CAKwvOd=accNK7t_SOmybo3e4UcBKoZ6TBPjCHT3eSSpSUouzEA@mail.gmail.com>
 <CADxym3Yxq0k_W43kVjrofjNoUUag3qwmpRGLLAQL1Emot3irPQ@mail.gmail.com> <20220818165838.GM25951@gate.crashing.org>
In-Reply-To: <20220818165838.GM25951@gate.crashing.org>
From:   Menglong Dong <menglong8.dong@gmail.com>
Date:   Fri, 19 Aug 2022 22:55:42 +0800
Message-ID: <CADxym3YEfSASDg9ppRKtZ16NLh_NhH253frd5LXZLGTObsVQ9g@mail.gmail.com>
Subject: Re: [PATCH net-next v4] net: skb: prevent the split of
 kfree_skb_reason() by gcc
To:     Segher Boessenkool <segher@kernel.crashing.org>
Cc:     Nick Desaulniers <ndesaulniers@google.com>, kuba@kernel.org,
        miguel.ojeda.sandonis@gmail.com, ojeda@kernel.org,
        davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
        asml.silence@gmail.com, imagedong@tencent.com,
        luiz.von.dentz@intel.com, vasily.averin@linux.dev,
        jk@codeconstruct.com.au, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, kernel test robot <lkp@intel.com>,
        linux-toolchains <linux-toolchains@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello,

On Fri, Aug 19, 2022 at 1:00 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Fri, Aug 19, 2022 at 12:31:44AM +0800, Menglong Dong wrote:
> > On Wed, Aug 17, 2022 at 11:54 PM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > > Perhaps noipa might also work here?
> >
> > In my testing, both 'noclone' and 'noipa' both work! As for the
> > '-fdisable-ipa-fnsplit', it seems it's not supported by gcc, and I
> > failed to find any documentation of it.
>
> noipa is noinline+noclone+no_icf plus assorted not separately enablable
> things.  There is no reason you would want to disable all
> inter-procedural optimisations here, so you don't need noipa.
>
> You need both noinline and no_icf if you want all calls to this to be
> actual function calls, and using this specific function name.  If you
> don't have noinline some calls may go missing (which may be fine for
> how you use it).  If you don't have no_icf the compiler may replace the
> call with a call to another function, if that does the same thing
> semantically.  You may want to prevent that as well, depending on
> exactly what you have this for.
>

Thanks for your explanation about the usage of 'noinline' and 'no_icf'!
I think 'noclone' seems enough in this case? As the function
'kfree_skb_reason' we talk about is a global function, I think that the
compiler has no reason to make it inline, or be merged with another
function.

Meanwhile, I think that the functions which use '__builtin_return_address'
should consider the optimization you mentioned above, and
I'll have a check on them by the way.

Thanks!
Menglong Dong

>
> Segher