public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: David Howells <dhowells@redhat.com>,
	Jeremy Fitzhardinge <jeremy@goop.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	tj@kernel.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] FRV: Fix the section attribute on UP DECLARE_PER_CPU()
Date: Wed, 15 Apr 2009 16:09:59 +0200	[thread overview]
Message-ID: <20090415140959.GD12760@elte.hu> (raw)
In-Reply-To: <1488.1239801659@redhat.com>


* David Howells <dhowells@redhat.com> wrote:

> > There's three too 'thick' headers: linux/percpu.h, 
> > linux/prefetch.h and asm/processor.h.
> 
> Yes, I noticed.
> 
> linux/percpu.h needs to be split three ways for instance: 
> definitions, access methods, and alocators.

yeah. Often a super-header has to be split into several basic 
headers or header pairs.

> linux/prefetch.h isn't too bad: what it needs is the prefetch 
> stuff splitting out of asm/processor.h into asm/prefetch.h.

yeah.

> > Please create include/linux/percpu_types.h for basic data types 
> > and simple, self-sufficient primitives. Also have an 
> > include/linux/percpu_api.h or include/linux/percpu.h include 
> > file for convenience/speedup inlines. The latter will only be 
> > included in .c files, where 'combination' of type spaces is not 
> > a problem.
> 
> Not so.  The problem is that various header files make use of 
> per-cpu variable accessors (asm/current.h and asm/thread_info.h to 
> name a couple) to build inline asm.

Hm, what portion did you mark with 'not so'?

inline asm is used in inline functions there, and that is what 
'instantiates' the types in a 'mixed' manner and way below their 
proper hierarchic level as well - creating both a mess and, as mess 
increases above a critical threshold an inevitable circular 
dependency as well.

> Anyway, here are a pair of patches on top of the one I've already 
> sent to Linus.  The second breaks a number of header files into 
> pieces and rearranges the percpu headers to put the DECLARE and 
> DEFINE macros together.
> 
> The first patch could potentially be applied immediately.  It adds 
> #inclusions and forward refs that are required to iron out compile 
> errors from the second patch.
> 
> Note that these only work for the configuration I routinely use on 
> my x86_64 test machine.  It will break all other arches and many 
> other i386 and x86_64 configurations.

I dont disagree, but i'd like to warn that such patches need _way_ 
more testing, these are never same-day obvious patches.

We have split up one big x86 header in this development cycle 
(asm/pgtable.h) and that alone was highly non-trivial and needed 
about a week to settle down, even with intense development and 
testing.

	Ingo

  reply	other threads:[~2009-04-15 14:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-14 16:10 [PATCH] FRV: Fix the section attribute on UP DECLARE_PER_CPU() David Howells
2009-04-14 16:57 ` Linus Torvalds
2009-04-14 17:57   ` David Howells
2009-04-14 18:06     ` Linus Torvalds
2009-04-15 10:24       ` David Howells
2009-04-15 15:19         ` Linus Torvalds
2009-04-15 16:49           ` Ingo Molnar
2009-04-15 11:40       ` David Howells
2009-04-15 12:03         ` Ingo Molnar
2009-04-15 13:20           ` David Howells
2009-04-15 14:09             ` Ingo Molnar [this message]
2009-04-15 15:09               ` David Howells
2009-04-15 16:54                 ` Ingo Molnar
2009-04-21 18:36                   ` David Howells
2009-04-22  9:12                     ` Ingo Molnar

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=20090415140959.GD12760@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox