public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

• * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
         [not found] ` <CANiq72kzHqQ-M2Gyrf7iy1BQMabM6txChYwa8hrDkMmc3w8ORw@mail.gmail.com>
  @ 2018-11-29  2:16   ` Xiaozhou Liu
    2018-11-29 10:42     ` Miguel Ojeda
    0 siblings, 1 reply; 6+ messages in thread
  From: Xiaozhou Liu @ 2018-11-29  2:16 UTC (permalink / raw)
    To: Miguel Ojeda
    Cc: netdev, linux-kernel, ast, daniel, yhs, jakub.kicinski,
  	quentin.monnet
  
  Hi Miguel,
  
  On Wed, Nov 28, 2018 at 06:35:18PM +0100, Miguel Ojeda wrote:
  > Hi Xiaozhou,
  > 
  > On Wed, Nov 28, 2018 at 3:09 PM Xiaozhou Liu <liuxiaozhou@bytedance•com> wrote:
  > >
  > > Attributes such as `__gnu_inline' are meant to be used within the
  > > kernel. When userspace somehow includes <linux/compiler.h>
  > > (eg. tools/bpf), compilation errors would be shown:
  > >
  > >         "error: unknown type name ‘__gnu_inline’"
  > >
  > > So just move these things into __KERNEL__ and the behavior is kept
  > > as before.
  
  By `these' I mean inline and the like, to be clear.
  
  > That is not exactly correct -- a3f8a30f3f00 moved some attributes to
  > another file, moving them into __KERNEL__ (in particular,__gnu_inline
  > is).
  
  Yes, that is what a3f8a30f3f00 did. Sorry.
  Turns out the commits in question are 815f0ddb346c and a3f8a30f3f00.
  
  > The problem is, instead, that __gnu_inline is not anymore defined
  > outside __KERNEL__, but something else that uses it is (the inline
  > macro definition, if I had to guess).
  
  Yes and no. Let's recall the whole story.
  
  Before 815f0ddb346c("include/linux/compiler*.h: make compiler-*.h mutually exclusive"),
  __gnu_inline and inline were both *in* __KERNEL__ as the were in
  <linux/compiler-gcc.h>, which was entirely put to __KERNEL__ in
  <linux/compiler_types.h>. Everything was fine.
  
  Then 815f0ddb346c moved inline and __gnu_inline *out* of __KERNEL__
  and put them in <linux/compiler_types.h> so userspace could see them
  both. Not sure if it's intended behavior, but everything looked fine.
  
  Then a3f8a30f3f00 moved __gnu_inline back into __KERNEL__ and left
  inline behind. Since inline depends on __gnu_inline, error showing
  "unknown type name ‘__gnu_inline’" pops up.
  
  > If your problem is fixed by putting __gnu_inline into __KERNEL__
  > again, it means we can simply move the inline definition inside
  > __KERNEL__ too. That way, we don't pollute userspace users with macro
  > definitions.
  
  It'd be good. That's exactly what my patch does.
  
  > Having said that, does someone know whether userspace should have
  > access to those attributes (or rather, other code that uses in turn
  > those attributes)?
  
  It'd be better to keep those attributes out of userspace, as is the
  case before 815f0ddb346c.
  
  BTW, the userspace code failed to compile in my case is under
  directory tools/bpf/.
  
  
  Thanks,
  Xiaozhou
  
  ^ permalink raw reply	[flat|nested] 6+ messages in thread

• • * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
      2018-11-29  2:16   ` [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__ Xiaozhou Liu
    @ 2018-11-29 10:42     ` Miguel Ojeda
      2018-12-06 12:39       ` Xiaozhou Liu
      0 siblings, 1 reply; 6+ messages in thread
    From: Miguel Ojeda @ 2018-11-29 10:42 UTC (permalink / raw)
      To: lxz1983
      Cc: Network Development, linux-kernel, ast, daniel, yhs,
    	jakub.kicinski, quentin.monnet
    
    On Thu, Nov 29, 2018 at 3:16 AM Xiaozhou Liu <lxz1983@gmail•com> wrote:
    >
    > On Wed, Nov 28, 2018 at 06:35:18PM +0100, Miguel Ojeda wrote:
    >
    > By `these' I mean inline and the like, to be clear.
    
    Ah, that makes more sense! Sorry.
    
    > > That is not exactly correct -- a3f8a30f3f00 moved some attributes to
    > > another file, moving them into __KERNEL__ (in particular,__gnu_inline
    > > is).
    >
    > Yes, that is what a3f8a30f3f00 did. Sorry.
    > Turns out the commits in question are 815f0ddb346c and a3f8a30f3f00.
    
    No problem! It was a bit confusing indeed.
    
    > Yes and no. Let's recall the whole story.
    >
    > Before 815f0ddb346c("include/linux/compiler*.h: make compiler-*.h mutually exclusive"),
    > __gnu_inline and inline were both *in* __KERNEL__ as the were in
    > <linux/compiler-gcc.h>, which was entirely put to __KERNEL__ in
    > <linux/compiler_types.h>. Everything was fine.
    >
    > Then 815f0ddb346c moved inline and __gnu_inline *out* of __KERNEL__
    > and put them in <linux/compiler_types.h> so userspace could see them
    > both. Not sure if it's intended behavior, but everything looked fine.
    >
    > Then a3f8a30f3f00 moved __gnu_inline back into __KERNEL__ and left
    > inline behind. Since inline depends on __gnu_inline, error showing
    > "unknown type name ‘__gnu_inline’" pops up.
    
    Exactly, thanks a lot for clarifying it up (we should put this in the
    commit message, I would say). That also answers my question: it is
    clear everything should be back into __KERNEL__. The only worry is
    that the v4.19 release contained 815f0ddb346c, so it has them exposed,
    so someone could have started relying on them. Or, more likely, the
    exposed macros could break some source code out there. Hm... Should a
    "fix" be backported?
    
    Cheers,
    Miguel
    
    ^ permalink raw reply	[flat|nested] 6+ messages in thread

  • • * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
        2018-11-29 10:42     ` Miguel Ojeda
      @ 2018-12-06 12:39       ` Xiaozhou Liu
        2018-12-06 12:49         ` Greg KH
        0 siblings, 1 reply; 6+ messages in thread
      From: Xiaozhou Liu @ 2018-12-06 12:39 UTC (permalink / raw)
        To: Miguel Ojeda; +Cc: Network Development, linux-kernel, gregkh
      
      On Thu, Nov 29, 2018 at 11:42:11AM +0100, Miguel Ojeda wrote:
      
      > Exactly, thanks a lot for clarifying it up (we should put this in the
      > commit message, I would say). That also answers my question: it is
      > clear everything should be back into __KERNEL__. The only worry is
      > that the v4.19 release contained 815f0ddb346c, so it has them exposed,
      > so someone could have started relying on them. Or, more likely, the
      > exposed macros could break some source code out there. Hm... Should a
      > "fix" be backported?
      
      What about letting v4.19 maintainers make the decision?
      
      Should I send a v2 patch updating the comments and Acked-by (you)?
      
      
      Thanks,
      Xiaozhou
      
      ^ permalink raw reply	[flat|nested] 6+ messages in thread

    • • * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
          2018-12-06 12:39       ` Xiaozhou Liu
        @ 2018-12-06 12:49         ` Greg KH
          2019-01-19 18:22           ` Miguel Ojeda
          0 siblings, 1 reply; 6+ messages in thread
        From: Greg KH @ 2018-12-06 12:49 UTC (permalink / raw)
          To: Xiaozhou Liu; +Cc: Miguel Ojeda, Network Development, linux-kernel
        
        On Thu, Dec 06, 2018 at 08:39:26PM +0800, Xiaozhou Liu wrote:
        > On Thu, Nov 29, 2018 at 11:42:11AM +0100, Miguel Ojeda wrote:
        > 
        > > Exactly, thanks a lot for clarifying it up (we should put this in the
        > > commit message, I would say). That also answers my question: it is
        > > clear everything should be back into __KERNEL__. The only worry is
        > > that the v4.19 release contained 815f0ddb346c, so it has them exposed,
        > > so someone could have started relying on them. Or, more likely, the
        > > exposed macros could break some source code out there. Hm... Should a
        > > "fix" be backported?
        > 
        > What about letting v4.19 maintainers make the decision?
        
        If something is fixed in Linus's tree for this, I want to take it into
        the 4.19-stable tree as well.
        
        thanks,
        
        greg k-h
        
        ^ permalink raw reply	[flat|nested] 6+ messages in thread

      • • * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
            2018-12-06 12:49         ` Greg KH
          @ 2019-01-19 18:22           ` Miguel Ojeda
            2019-01-21 10:43             ` Greg KH
            0 siblings, 1 reply; 6+ messages in thread
          From: Miguel Ojeda @ 2019-01-19 18:22 UTC (permalink / raw)
            To: Greg KH, Nick Desaulniers, Xiaozhou Liu; +Cc: Network Development, linux-kernel
          
          Hi Greg, Nick, Xiaozhou,
          
          On Thu, Dec 6, 2018 at 1:50 PM Greg KH <gregkh@linuxfoundation•org> wrote:
          >
          > If something is fixed in Linus's tree for this, I want to take it into
          > the 4.19-stable tree as well.
          
          This ended up in Linus' tree before the holidays, i.e. 4.20 has it,
          see commit 71391bdd2e9a ("include/linux/compiler_types.h: don't
          pollute userspace with macro definitions").
          
          In case you want to still backport this to 4.19: you can't cherry-pick
          it without conflicts because some stuff was moved around due to the
          Compiler Attributes patch series (which also went in with 4.20 too),
          but you can move the macros like this commit does. There are 2
          conflicts:
          
            * The big block of macros for attributes:
          https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/compiler_types.h?h=v4.19.16#n189
            * The __always_inline macro:
          https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/compiler_types.h?h=v4.19.16#n275
          
          All those are nowadays inside __KERNEL__ && !__ASSEMBLY__, so it
          should be fine to move those too along the rest that this patch moves.
          
          Cheers,
          Miguel
          
          ^ permalink raw reply	[flat|nested] 6+ messages in thread

        • • * Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__
              2019-01-19 18:22           ` Miguel Ojeda
            @ 2019-01-21 10:43             ` Greg KH
              0 siblings, 0 replies; 6+ messages in thread
            From: Greg KH @ 2019-01-21 10:43 UTC (permalink / raw)
              To: Miguel Ojeda
              Cc: Nick Desaulniers, Xiaozhou Liu, Network Development, linux-kernel
            
            On Sat, Jan 19, 2019 at 07:22:50PM +0100, Miguel Ojeda wrote:
            > Hi Greg, Nick, Xiaozhou,
            > 
            > On Thu, Dec 6, 2018 at 1:50 PM Greg KH <gregkh@linuxfoundation•org> wrote:
            > >
            > > If something is fixed in Linus's tree for this, I want to take it into
            > > the 4.19-stable tree as well.
            > 
            > This ended up in Linus' tree before the holidays, i.e. 4.20 has it,
            > see commit 71391bdd2e9a ("include/linux/compiler_types.h: don't
            > pollute userspace with macro definitions").
            > 
            > In case you want to still backport this to 4.19: you can't cherry-pick
            > it without conflicts because some stuff was moved around due to the
            > Compiler Attributes patch series (which also went in with 4.20 too),
            > but you can move the macros like this commit does. There are 2
            > conflicts:
            > 
            >   * The big block of macros for attributes:
            > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/compiler_types.h?h=v4.19.16#n189
            >   * The __always_inline macro:
            > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/compiler_types.h?h=v4.19.16#n275
            > 
            > All those are nowadays inside __KERNEL__ && !__ASSEMBLY__, so it
            > should be fine to move those too along the rest that this patch moves.
            
            I have no idea why I would want to backport this, sorry :(
            
            If this resolves a problem, great, but someone has to do the backport
            for me to be able to take it.
            
            thanks,
            
            greg k-h
            
            > 
            > Cheers,
            > Miguel
            
            ^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-21 10:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20181128140852.178260-1-liuxiaozhou@bytedance.com>
     [not found] ` <CANiq72kzHqQ-M2Gyrf7iy1BQMabM6txChYwa8hrDkMmc3w8ORw@mail.gmail.com>
2018-11-29  2:16   ` [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__ Xiaozhou Liu
2018-11-29 10:42     ` Miguel Ojeda
2018-12-06 12:39       ` Xiaozhou Liu
2018-12-06 12:49         ` Greg KH
2019-01-19 18:22           ` Miguel Ojeda
2019-01-21 10:43             ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox