Messages in this thread | | | From | Nick Desaulniers <> | Date | Wed, 17 Aug 2022 08:54:44 -0700 | Subject | Re: [PATCH net-next v4] net: skb: prevent the split of kfree_skb_reason() by gcc |
| |
On Mon, Aug 15, 2022 at 8:29 PM <menglong8.dong@gmail.com> wrote: > > From: Menglong Dong <imagedong@tencent.com> > > Sometimes, gcc will optimize the function by spliting it to two or > more functions. In this case, kfree_skb_reason() is splited to > kfree_skb_reason and kfree_skb_reason.part.0. However, the > function/tracepoint trace_kfree_skb() in it needs the return address > of kfree_skb_reason().
Does the existing __noclone function attribute help at all here?
If not, surely there's an attribute that's more precise than "disable most optimization outright."
https://unix.stackexchange.com/questions/223013/function-symbol-gets-part-suffix-after-compilation https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute
Perhaps noipa might also work here?
> > This split makes the call chains becomes: > kfree_skb_reason() -> kfree_skb_reason.part.0 -> trace_kfree_skb() > > which makes the return address that passed to trace_kfree_skb() be > kfree_skb(). > > Therefore, prevent this kind of optimization to kfree_skb_reason() by > making the optimize level to "O1". I think these should be better > method instead of this "O1", but I can't figure it out...... > > This optimization CAN happen, which depend on the behavior of gcc. > I'm not able to reproduce it in the latest kernel code, but it happens > in my kernel of version 5.4.119. Maybe the latest code already do someting > that prevent this happen? > > Signed-off-by: Menglong Dong <imagedong@tencent.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> > --- > v4: > - move the definition of __nofnsplit to compiler_attributes.h > > v3: > - define __nofnsplit only for GCC > - add some document > > v2: > - replace 'optimize' with '__optimize__' in __nofnsplit, as Miguel Ojeda > advised. > --- > include/linux/compiler_attributes.h | 19 +++++++++++++++++++ > net/core/skbuff.c | 3 ++- > 2 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h > index 445e80517cab..968cbafa2421 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -270,6 +270,25 @@ > */ > #define __noreturn __attribute__((__noreturn__)) > > +/* > + * Optional: not supported by clang. > + * Optional: not supported by icc. > + * > + * Prevent function from being splited to multiple part. As what the > + * document says in gcc/ipa-split.cc, single function will be splited > + * when necessary: > + * > + * https://github.com/gcc-mirror/gcc/blob/master/gcc/ipa-split.cc > + * > + * This optimization seems only take effect on O2 and O3 optimize level. > + * Therefore, make the optimize level to O1 to prevent this optimization. > + */ > +#if __has_attribute(__optimize__) > +# define __nofnsplit __attribute__((__optimize__("O1"))) > +#else > +# define __nofnsplit > +#endif > + > /* > * Optional: not supported by gcc. > * Optional: not supported by icc. > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 974bbbbe7138..ff9ccbc032b9 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -777,7 +777,8 @@ EXPORT_SYMBOL(__kfree_skb); > * hit zero. Meanwhile, pass the drop reason to 'kfree_skb' > * tracepoint. > */ > -void kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason) > +void __nofnsplit > +kfree_skb_reason(struct sk_buff *skb, enum skb_drop_reason reason) > { > if (!skb_unref(skb)) > return; > -- > 2.36.1 >
-- Thanks, ~Nick Desaulniers
| |