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> <CADxym3YEfSASDg9ppRKtZ16NLh_NhH253frd5LXZLGTObsVQ9g@mail.gmail.com>
 <20220819152157.GO25951@gate.crashing.org>
In-Reply-To: <20220819152157.GO25951@gate.crashing.org>
From:   Menglong Dong <menglong8.dong@gmail.com>
Date:   Sat, 20 Aug 2022 19:00:41 +0800
Message-ID: <CADxym3Y-=6pRP=CunxRomfwXf58k0LyLm510WGtzsBnzjqdD4g@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 11:24 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Fri, Aug 19, 2022 at 10:55:42PM +0800, Menglong Dong wrote:
> > 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.
>
> Whether something is inlined is decided per instance (except for
> always_inline and noinline functions).  Of course the function body has
> to be available for anything to be inlined, so barring LTO this can only
> happen for function uses in the same source file.  Not very likely
> indeed, but not entirely impossible either.
>

I understand it now, the global function is indeed possible to be
made inline by the compiler, and 'noinline' seems necessary
here too.

Maybe I can add a new compiler attribute like this:

/*
 * Used by functions that use '__builtin_return_address'. These function
 * don't want to be splited or made inline, which can make
 * the '__builtin_return_address' got unexpected address.
 */
#define __fix_address noinline __noclone

> A function can be merged if there is another function that does exactly
> the same thing.  This is unlikely with functions that do some serious
> work of course, but it is likely with stub-like functions.
>

I understand how 'icf'(Identical Code Folding) works now. In the case
we talk about, It seems fine even if the function is merged. The only
effect of 'icf' is the change of function name, which doesn't affect
the result of '__builtin_return_address(0)'.

Thanks!
Menglong Dong

> gl;hf,
>
>
> Segher