All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Jaswinder Singh Rajput <jaswinder@kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Jaswinder Singh Rajput <jaswinderlinux@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	hskinnemoen@atmel.com, cooloney@kernel.org, tony.luck@intel.com,
	ralf@linux-mips.org, dhowells@redhat.com, matthew@wil.cx,
	chris@zankel.net, LKML <linux-kernel@vger.kernel.org>,
	linux-next <linux-next@vger.kernel.org>,
	linux-ia64 <linux-ia64@vger.kernel.org>
Subject: Re: [linux-next][PATCH] revert headers_check fix: ia64, fpu.h
Date: Fri, 06 Feb 2009 17:32:49 +0000	[thread overview]
Message-ID: <20090206173249.GC11299@uranus.ravnborg.org> (raw)
In-Reply-To: <20090206155512.GF13758@n2100.arm.linux.org.uk>

On Fri, Feb 06, 2009 at 03:55:12PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 06, 2009 at 09:18:48PM +0530, Jaswinder Singh Rajput wrote:
> > On Fri, 2009-02-06 at 15:33 +0000, Russell King - ARM Linux wrote:
> > > On Fri, Feb 06, 2009 at 08:59:01PM +0530, Jaswinder Singh Rajput wrote:
> > > > Jaswinder Singh Rajput (2):
> > > >       Neither asm/types.h nor linux/types.h is required for arch/ia64/include/asm/fpu.h
> > > >       make linux/types.h as assembly safe
> > > 
> > > I continue to disagree with the need for the second patch.
> > 
> > Like Ingo suggested:
> > 
> > On Fri, 2009-02-06 at 15:58 +0100, Ingo Molnar wrote:
> > >  Well types.h easily gets included in other files though, which might be 
> > > partially suited for assembly - and have !__ASSEMBLY__ portions that rely on 
> > > a types.h include.
> > > 
> > > So making this file an invariant in .S files does not sound like a bad idea 
> > > to me. Is there any downside?
> > > 
> > 
> > We cannot see any downside of this patch.
> > 
> > But we can see upside of this patch is:
> > 1. No need to protect linux/types.h with #ifndef __ASSEMBLY__ in many
> > files
> > 2. So we trying to replace multiple #ifndef __ASSEMBLY__ with one.
> 
> The point is:
> 
> 1. If the parent include needs to include linux/types.h to get at C
>    types _and_ the include file needs to also be included by assembly
>    code, it itself needs to have #ifndef __ASSEMBLY__ to protect those
>    uses from the assembly code.
> 
>    In that case, the linux/types.h include should be contained within
>    the #ifndef __ASSEMBLY__ .. #endif block along with all C only
>    parts of the header file.
> 
> 2. if it doesn't need C types from linux/types.h, then that header has
>    no business including linux/types.h, and the include should be
>    eliminated to save the already dirbolically slow compiler from
>    having to read and parse that file, and more importantly allowing
>    it to eliminate linux/types.h from the build dependencies.
> 
> Yes, you can wrap linux/types.h with that ifndef, and yes it will fix
> any problems, but I view it as a hack rather than fixing the real problem
> which is lazyness by code writers to get their include dependencies right.

You guys are getting this wrong.
The patch from Jaswinder needs to be fixed so we unconditionally
include <asm/types.h> from linux/types.h.
And then ll users can safely include linux/types.h and when we one
day realize we can move some stuff used in .S files from
asm/types.h to linux/types.h then we are all safe and no breakage.

the rule of thum is to include the linux/* variant if the same
file exist in both linus/ and asm/ and types.h is in no way different
here

And trying to make it different just becasue it is used in userspace
intensively is just stupid and will be a cause of misunderstandings.

	Sam

WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Jaswinder Singh Rajput <jaswinder@kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Jaswinder Singh Rajput <jaswinderlinux@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	hskinnemoen@atmel.com, cooloney@kernel.org, tony.luck@intel.com,
	ralf@linux-mips.org, dhowells@redhat.com, matthew@wil.cx,
	chris@zankel.net, LKML <linux-kernel@vger.kernel.org>,
	linux-next <linux-next@vger.kernel.org>,
	linux-ia64 <linux-ia64@vger.kernel.org>
Subject: Re: [linux-next][PATCH] revert headers_check fix: ia64, fpu.h
Date: Fri, 6 Feb 2009 18:32:49 +0100	[thread overview]
Message-ID: <20090206173249.GC11299@uranus.ravnborg.org> (raw)
In-Reply-To: <20090206155512.GF13758@n2100.arm.linux.org.uk>

On Fri, Feb 06, 2009 at 03:55:12PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 06, 2009 at 09:18:48PM +0530, Jaswinder Singh Rajput wrote:
> > On Fri, 2009-02-06 at 15:33 +0000, Russell King - ARM Linux wrote:
> > > On Fri, Feb 06, 2009 at 08:59:01PM +0530, Jaswinder Singh Rajput wrote:
> > > > Jaswinder Singh Rajput (2):
> > > >       Neither asm/types.h nor linux/types.h is required for arch/ia64/include/asm/fpu.h
> > > >       make linux/types.h as assembly safe
> > > 
> > > I continue to disagree with the need for the second patch.
> > 
> > Like Ingo suggested:
> > 
> > On Fri, 2009-02-06 at 15:58 +0100, Ingo Molnar wrote:
> > >  Well types.h easily gets included in other files though, which might be 
> > > partially suited for assembly - and have !__ASSEMBLY__ portions that rely on 
> > > a types.h include.
> > > 
> > > So making this file an invariant in .S files does not sound like a bad idea 
> > > to me. Is there any downside?
> > > 
> > 
> > We cannot see any downside of this patch.
> > 
> > But we can see upside of this patch is:
> > 1. No need to protect linux/types.h with #ifndef __ASSEMBLY__ in many
> > files
> > 2. So we trying to replace multiple #ifndef __ASSEMBLY__ with one.
> 
> The point is:
> 
> 1. If the parent include needs to include linux/types.h to get at C
>    types _and_ the include file needs to also be included by assembly
>    code, it itself needs to have #ifndef __ASSEMBLY__ to protect those
>    uses from the assembly code.
> 
>    In that case, the linux/types.h include should be contained within
>    the #ifndef __ASSEMBLY__ .. #endif block along with all C only
>    parts of the header file.
> 
> 2. if it doesn't need C types from linux/types.h, then that header has
>    no business including linux/types.h, and the include should be
>    eliminated to save the already dirbolically slow compiler from
>    having to read and parse that file, and more importantly allowing
>    it to eliminate linux/types.h from the build dependencies.
> 
> Yes, you can wrap linux/types.h with that ifndef, and yes it will fix
> any problems, but I view it as a hack rather than fixing the real problem
> which is lazyness by code writers to get their include dependencies right.

You guys are getting this wrong.
The patch from Jaswinder needs to be fixed so we unconditionally
include <asm/types.h> from linux/types.h.
And then ll users can safely include linux/types.h and when we one
day realize we can move some stuff used in .S files from
asm/types.h to linux/types.h then we are all safe and no breakage.

the rule of thum is to include the linux/* variant if the same
file exist in both linus/ and asm/ and types.h is in no way different
here

And trying to make it different just becasue it is used in userspace
intensively is just stupid and will be a cause of misunderstandings.

	Sam

  parent reply	other threads:[~2009-02-06 17:32 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-31  7:10 [git pull -tip] headers_check fixes for other architectures Jaswinder Singh Rajput
2009-01-31 16:44 ` Ingo Molnar
2009-02-01  6:30   ` Jaswinder Singh Rajput
2009-02-01 10:39     ` Ingo Molnar
2009-02-01 16:49       ` Ingo Molnar
2009-02-01 17:06         ` Russell King - ARM Linux
2009-02-01 17:20         ` Russell King - ARM Linux
2009-02-01 17:31           ` Jaswinder Singh Rajput
2009-02-01 18:33         ` Jaswinder Singh Rajput
2009-02-02 17:11         ` Mike Frysinger
2009-02-02 18:08           ` Ingo Molnar
2009-02-02 18:29             ` Mike Frysinger
2009-02-02 18:48               ` Ingo Molnar
2009-02-05 17:55 ` Tony Luck
2009-02-05 19:06   ` Linus Torvalds
2009-02-05 19:19     ` Ingo Molnar
2009-02-06  2:06       ` Jaswinder Singh Rajput
2009-02-06  2:14         ` Mike Frysinger
2009-02-06  2:20         ` Ingo Molnar
2009-02-06 14:18           ` Jaswinder Singh Rajput
2009-02-06 14:21             ` Russell King - ARM Linux
2009-02-06 14:34               ` Jaswinder Singh Rajput
2009-02-06 14:51                 ` Russell King - ARM Linux
2009-02-06 14:58                   ` Ingo Molnar
2009-02-06 15:00                   ` Jaswinder Singh Rajput
2009-02-06 15:10                     ` Russell King - ARM Linux
2009-02-06 15:56                       ` Ingo Molnar
2009-02-06 16:02                         ` Russell King - ARM Linux
2009-02-06 17:24             ` Sam Ravnborg
2009-02-06 17:40               ` Ingo Molnar
2009-02-06 18:11               ` Jaswinder Singh Rajput
2009-02-06 18:42                 ` Sam Ravnborg
2009-02-06 19:31                   ` Jaswinder Singh Rajput
2009-02-08  5:50               ` Jaswinder Singh Rajput
2009-02-09 12:23                 ` Ingo Molnar
2009-02-06  8:19 ` [linux-next][PATCH] revert headers_check fix: ia64, fpu.h KOSAKI Motohiro
2009-02-06  8:19   ` KOSAKI Motohiro
2009-02-06  8:19   ` KOSAKI Motohiro
2009-02-06  8:53   ` [PATCH] " Jaswinder Singh Rajput
2009-02-06  8:53     ` [linux-next][PATCH] " Jaswinder Singh Rajput
2009-02-06  8:53     ` Jaswinder Singh Rajput
2009-02-06  9:11     ` KOSAKI Motohiro
2009-02-06  9:11       ` KOSAKI Motohiro
2009-02-06 14:55       ` Ingo Molnar
2009-02-06 14:55         ` Ingo Molnar
2009-02-06 14:55         ` [PATCH] " Ingo Molnar
2009-02-06 15:29         ` [linux-next][PATCH] " Jaswinder Singh Rajput
2009-02-06 15:41           ` Jaswinder Singh Rajput
2009-02-06 15:33           ` Russell King - ARM Linux
2009-02-06 15:33             ` Russell King - ARM Linux
2009-02-06 15:45             ` Ingo Molnar
2009-02-06 15:45               ` Ingo Molnar
2009-02-06 15:45               ` [PATCH] " Ingo Molnar
2009-02-06 15:49               ` [linux-next][PATCH] " Russell King - ARM Linux
2009-02-06 15:49                 ` Russell King - ARM Linux
2009-02-06 15:49                 ` [PATCH] " Russell King - ARM Linux
2009-02-06 16:01                 ` [linux-next][PATCH] " Ingo Molnar
2009-02-06 16:01                   ` Ingo Molnar
2009-02-06 16:01                   ` [PATCH] " Ingo Molnar
2009-02-06 15:48             ` Jaswinder Singh Rajput
2009-02-06 15:52               ` [linux-next][PATCH] " Jaswinder Singh Rajput
2009-02-06 15:48               ` Jaswinder Singh Rajput
2009-02-06 15:55               ` Russell King - ARM Linux
2009-02-06 15:55                 ` Russell King - ARM Linux
2009-02-06 16:12                 ` Ingo Molnar
2009-02-06 16:12                   ` Ingo Molnar
2009-02-06 16:23                   ` Russell King - ARM Linux
2009-02-06 16:23                     ` Russell King - ARM Linux
2009-02-06 16:33                     ` Ingo Molnar
2009-02-06 16:33                       ` Ingo Molnar
2009-02-06 16:33                       ` [PATCH] " Ingo Molnar
2009-02-06 16:38                       ` [linux-next][PATCH] " Russell King - ARM Linux
2009-02-06 16:38                         ` Russell King - ARM Linux
2009-02-06 16:38                         ` [PATCH] " Russell King - ARM Linux
2009-02-06 17:14                         ` [linux-next][PATCH] " Ingo Molnar
2009-02-06 17:14                           ` Ingo Molnar
2009-02-06 17:22                           ` Russell King - ARM Linux
2009-02-06 17:22                             ` Russell King - ARM Linux
2009-02-06 17:32                 ` Sam Ravnborg [this message]
2009-02-06 17:32                   ` Sam Ravnborg
2009-02-06 17:41                   ` Ingo Molnar
2009-02-06 17:41                     ` Ingo Molnar
2009-02-06 18:53     ` Luck, Tony
2009-02-06 18:53       ` Luck, Tony
2009-02-06 18:53       ` [PATCH] " Luck, Tony
2009-02-06 13:42   ` [linux-next][PATCH] " Sam Ravnborg
2009-02-06 13:42     ` Sam Ravnborg
2009-02-06 13:42     ` [PATCH] " Sam Ravnborg

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=20090206173249.GC11299@uranus.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=akpm@linux-foundation.org \
    --cc=chris@zankel.net \
    --cc=cooloney@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=hskinnemoen@atmel.com \
    --cc=jaswinder@kernel.org \
    --cc=jaswinderlinux@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=matthew@wil.cx \
    --cc=mingo@elte.hu \
    --cc=ralf@linux-mips.org \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    /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.