Re: [PATCH] arch/tile: Add driver to enable access to the user dynamic network.

Re: [PATCH] arch/tile: Add driver to enable access to the user dynamic network.

[Arnd Bergmann]

Cc'ing linux-arch on the question of how to use a list_head in
processor.h.

On Monday 28 June 2010 17:23:16 Chris Metcalf wrote:
> > We could break the dependency by turning prefetch_range into a macro
> > or an extern function. There is only one user, and it's in a staging
> > driver, so the impact would be minimal.
> >   
> 
> I don't think so.  The problem is that users of <linux/list.h> expect to
> be able to #include that one header, then use things like
> list_for_each() (which uses prefetch, as defined in <asm/processor.h>),
> but without also being required to #include <asm/processor.h> themselves
> explicitly.

Right.
 
> I think the only "true" fix would be to have a new <linux/list_types.h>
> header that provides list_head (and presumably hlist_head and
> hlist_node), which <linux/list.h> would include, as would our
> <asm/processor.h>.  This is certainly in line with recent
> header-separation changes (e.g. mm_types.h).  Would there be interest in
> a change like this?  I implemented it in my tree, and if it sounds
> plausible to you, I'll send out a git diff, but it looks pretty much
> exactly like this description :-)

Yes, I think that would be a reasonable change.

Another alternative might be to move the prefetch stuff from asm/processor.h
to asm/prefetch.h on all architectures, which also breaks the dependency loop,
unless I'm mistaken again.

	Arnd

Re: [PATCH] arch/tile: Add driver to enable access to the user dynamic network.

[Chris Metcalf]

[The question is how to use <linux/list.h> from within <asm/processor.h>.]

On 6/28/2010 3:34 PM, Arnd Bergmann wrote:
>> I think the only "true" fix would be to have a new <linux/list_types.h>
>> > header that provides list_head (and presumably hlist_head and
>> > hlist_node), which <linux/list.h> would include, as would our
>> > <asm/processor.h>.  This is certainly in line with recent
>> > header-separation changes (e.g. mm_types.h).  Would there be interest in
>> > a change like this?  I implemented it in my tree, and if it sounds
>> > plausible to you, I'll send out a git diff, but it looks pretty much
>> > exactly like this description :-)
>>     
> Yes, I think that would be a reasonable change.
>
> Another alternative might be to move the prefetch stuff from asm/processor.h
> to asm/prefetch.h on all architectures, which also breaks the dependency loop,
> unless I'm mistaken again

In principle I like the <asm/prefetch.h> idea, but I'm concerned that
the #include of <asm/system.h> from <linux/list.h> will recursively
include <asm/processor.h> on some platforms; for example, s390 and
xtensa include it directly.  We (tile) were including it indirectly via
<asm/irqflags.h>, though this seems to be a spurious include on our
part, but other platforms may also include it indirectly.  To be fair,
I'm not sure why <asm/system.h> is included from <linux/list.h>.  It
doesn't seem required for a tile build, at least, but no doubt it was
put there for some reason.

So, if there's a good reason for it to be there, I'd say that pushes us
back toward a separate <linux/list_types.h>.  Otherwise we can
investigate splitting out the prefetch content on every platform to
<asm/prefetch.h> (presumably creating some empty <asm/prefetch.h>
headers on architectures that just use the gcc builtin) and adding new
#includes of <asm/prefetch.h> to files that reference the prefetch
functionality.  Arnd and other list folks, what's your instinct?

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] arch/tile: Add driver to enable access to the user dynamic network.

[Arnd Bergmann]

On Friday 02 July 2010, Chris Metcalf wrote:
> So, if there's a good reason for it to be there, I'd say that pushes us
> back toward a separate <linux/list_types.h>.  Otherwise we can
> investigate splitting out the prefetch content on every platform to
> <asm/prefetch.h> (presumably creating some empty <asm/prefetch.h>
> headers on architectures that just use the gcc builtin) and adding new
> #includes of <asm/prefetch.h> to files that reference the prefetch
> functionality.  Arnd and other list folks, what's your instinct?

Makes sense. Splitting out the list types from list.h does seem to be
safest option. We might actually be able to do some header file
untangling that way, by using list_types.h in all headers that
use a list_head by none of the macros and functions associated with it.

	Arnd

[PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Chris Metcalf]

This allows a list_head (or hlist_head, etc.) to be used from places
that used to be impractical, in particular <asm/processor.h>, which
used to cause include file recursion: <linux/list.h> includes
<linux/prefetch.h>, which always includes <asm/processor.h> for the
prefetch macros, as well as <asm/system.h>, which often includes
<asm/processor.h> directly or indirectly.

This avoids a lot of painful workaround hackery on the tile
architecture, where we use a list_head in the thread_struct to chain
together tasks that are activated on a particular hardwall.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---

 include/linux/list.h       |   13 +------------
 include/linux/list_types.h |   16 ++++++++++++++++
 2 files changed, 17 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/list_types.h

diff --git a/include/linux/list.h b/include/linux/list.h
index 8392884..3a42003 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -4,6 +4,7 @@
 #include <linux/stddef.h>
 #include <linux/poison.h>
 #include <linux/prefetch.h>
+#include <linux/list_types.h>
 #include <asm/system.h>
 
 /*
@@ -16,10 +17,6 @@
  * using the generic single-entry routines.
  */
 
-struct list_head {
-	struct list_head *next, *prev;
-};
-
 #define LIST_HEAD_INIT(name) { &(name), &(name) }
 
 #define LIST_HEAD(name) \
@@ -551,14 +548,6 @@ static inline void list_splice_tail_init(struct list_head *list,
  * You lose the ability to access the tail in O(1).
  */
 
-struct hlist_head {
-	struct hlist_node *first;
-};
-
-struct hlist_node {
-	struct hlist_node *next, **pprev;
-};
-
 #define HLIST_HEAD_INIT { .first = NULL }
 #define HLIST_HEAD(name) struct hlist_head name = {  .first = NULL }
 #define INIT_HLIST_HEAD(ptr) ((ptr)->first = NULL)
diff --git a/include/linux/list_types.h b/include/linux/list_types.h
new file mode 100644
index 0000000..c991e28
--- /dev/null
+++ b/include/linux/list_types.h
@@ -0,0 +1,16 @@
+#ifndef _LINUX_LIST_TYPES_H
+#define _LINUX_LIST_TYPES_H
+
+struct list_head {
+	struct list_head *next, *prev;
+};
+
+struct hlist_head {
+	struct hlist_node *first;
+};
+
+struct hlist_node {
+	struct hlist_node *next, **pprev;
+};
+
+#endif
-- 
1.6.5.2

Re: [PATCH] arch/tile: Add driver to enable access to the user dynamic network.

[Chris Metcalf]

On 7/2/2010 12:11 PM, Arnd Bergmann wrote:
> On Friday 02 July 2010, Chris Metcalf wrote:
>   
>> So, if there's a good reason for it to be there, I'd say that pushes us
>> back toward a separate <linux/list_types.h>.  Otherwise we can
>> investigate splitting out the prefetch content on every platform to
>> <asm/prefetch.h> (presumably creating some empty <asm/prefetch.h>
>> headers on architectures that just use the gcc builtin) and adding new
>> #includes of <asm/prefetch.h> to files that reference the prefetch
>> functionality.  Arnd and other list folks, what's your instinct?
>>     
> Makes sense. Splitting out the list types from list.h does seem to be
> safest option. We might actually be able to do some header file
> untangling that way, by using list_types.h in all headers that
> use a list_head by none of the macros and functions associated with it.
>   

For now I'll just stick with the straight splitting-out (see recent git
email).  There may be kernel code that is getting the list macros and
functions "by accident" by including some header that in itself only
needs the structs and so could use <linux/list_types.h>, and would need
to #include <linux/list.h> itself to avoid breaking.  There would
probably be a long tail of complaints that developers' obscure
architecture, driver, etc., had been broken by an aggressive
"untangling" change.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Matthew Wilcox]

On Fri, Jul 02, 2010 at 01:41:14PM -0400, Chris Metcalf wrote:
> This allows a list_head (or hlist_head, etc.) to be used from places
> that used to be impractical, in particular <asm/processor.h>, which
> used to cause include file recursion: <linux/list.h> includes
> <linux/prefetch.h>, which always includes <asm/processor.h> for the
> prefetch macros, as well as <asm/system.h>, which often includes
> <asm/processor.h> directly or indirectly.

Why a new header file instead of linux/types.h?

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Chris Metcalf]

On 7/2/2010 3:19 PM, Matthew Wilcox wrote:
> On Fri, Jul 02, 2010 at 01:41:14PM -0400, Chris Metcalf wrote:
>   
>> This allows a list_head (or hlist_head, etc.) to be used from places
>> that used to be impractical, in particular <asm/processor.h>, which
>> used to cause include file recursion: <linux/list.h> includes
>> <linux/prefetch.h>, which always includes <asm/processor.h> for the
>> prefetch macros, as well as <asm/system.h>, which often includes
>> <asm/processor.h> directly or indirectly.
>>     
> Why a new header file instead of linux/types.h?
>   

I was working from analogy to kvm_types.h, mm_types.h, rwlock_types.h,
spinlock_types.h.  My impression is that linux/types.h is generally for
basic (non-struct) types, with atomic_t/atomic64_t being added as
"almost non-struct types", and of course the historical exception of
"struct ustat", which has been there since the dawn of time (0.97 anyway).

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Arnd Bergmann]

On Friday 02 July 2010 21:19:11 Matthew Wilcox wrote:
> Why a new header file instead of linux/types.h?

I think it mostly makes sense because a list_head by itself usually
isn't all that useful, you also want the list_add/list_for_each/...
macros, so you end up including linux/list.h anyway.

linux/list_types.h is really a special case which can get included
by other headers when they have a reason for doing that.

	Arnd

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Matthew Wilcox]

On Fri, Jul 02, 2010 at 03:33:52PM -0400, Chris Metcalf wrote:
> On 7/2/2010 3:19 PM, Matthew Wilcox wrote:
> > Why a new header file instead of linux/types.h?
> 
> I was working from analogy to kvm_types.h, mm_types.h, rwlock_types.h,
> spinlock_types.h.  My impression is that linux/types.h is generally for
> basic (non-struct) types, with atomic_t/atomic64_t being added as
> "almost non-struct types", and of course the historical exception of
> "struct ustat", which has been there since the dawn of time (0.97 anyway).

I think list_head, hlist_head and hlist_node qualify as "almost non-struct
types", don't you?  :-)

I wouldn't mind seeing kvm_types.h, rwlock_types.h and spinlock_types.h
merged into types.h, personally.  They're all pretty fundamental kernel
kind of types.  It's a matter of taste, and I'm not particularly fussed
one way or the other.

mm_types.h is complex and full of mm-specific information, so keeping
it separate makes sense to me.

I just object to the unnecessary creation of tiny files like this.
Which is how we ended up with atomic_t and atomic64_t in there in the
first place :-)

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Chris Metcalf]

On 7/2/2010 4:48 PM, Matthew Wilcox wrote:
> On Fri, Jul 02, 2010 at 03:33:52PM -0400, Chris Metcalf wrote:
>   
>> On 7/2/2010 3:19 PM, Matthew Wilcox wrote:
>>     
>>> Why a new header file instead of linux/types.h?
>>>       
>> I was working from analogy to kvm_types.h, mm_types.h, rwlock_types.h,
>> spinlock_types.h.  My impression is that linux/types.h is generally for
>> basic (non-struct) types, with atomic_t/atomic64_t being added as
>> "almost non-struct types", and of course the historical exception of
>> "struct ustat", which has been there since the dawn of time (0.97 anyway).
>>     
> I think list_head, hlist_head and hlist_node qualify as "almost non-struct
> types", don't you?  :-)
>   

I see the smiley, but to reply seriously, the distinction I was making
was that atomic_t is really just an integer type, but with typing magic
to protect it from implicit conversion -- unlike list_head, which really
is a more complex type.

I suppose one could make a kind of "intent of the founders"
constitutional law-type argument suggesting that the presence of "struct
ustat" suggests more complex types are in fact appropriate in
<linux/types.h>.  :-)

> I wouldn't mind seeing kvm_types.h, rwlock_types.h and spinlock_types.h
> merged into types.h, personally.  They're all pretty fundamental kernel
> kind of types.  It's a matter of taste, and I'm not particularly fussed
> one way or the other.
>   

Somehow it's hard to see kvm_ioapic_redirect_entry on a par with size_t :-)

> I just object to the unnecessary creation of tiny files like this.
> Which is how we ended up with atomic_t and atomic64_t in there in the
> first place :-)
>   

In any case, I think this either way is plausible, but in the absence of
more folks weighing in, I think "avoid adding a complex type to
<linux/types.h>" sounds more convincing to me than "avoid adding a new
tiny file", though I certainly do buy the latter argument.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Christoph Hellwig]

On Fri, Jul 02, 2010 at 10:43:13PM +0200, Arnd Bergmann wrote:
> On Friday 02 July 2010 21:19:11 Matthew Wilcox wrote:
> > Why a new header file instead of linux/types.h?
> 
> I think it mostly makes sense because a list_head by itself usually
> isn't all that useful, you also want the list_add/list_for_each/...
> macros, so you end up including linux/list.h anyway.

It's useful for headers.  You can assume linux/types.h is already
included and don't have to bother to include list{_types}.h everywhere.

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Alexey Dobriyan]

On Fri, Jul 02, 2010 at 02:48:17PM -0600, Matthew Wilcox wrote:
> On Fri, Jul 02, 2010 at 03:33:52PM -0400, Chris Metcalf wrote:
> > On 7/2/2010 3:19 PM, Matthew Wilcox wrote:
> > > Why a new header file instead of linux/types.h?
> > 
> > I was working from analogy to kvm_types.h, mm_types.h, rwlock_types.h,
> > spinlock_types.h.  My impression is that linux/types.h is generally for
> > basic (non-struct) types, with atomic_t/atomic64_t being added as
> > "almost non-struct types", and of course the historical exception of
> > "struct ustat", which has been there since the dawn of time (0.97 anyway).
> 
> I think list_head, hlist_head and hlist_node qualify as "almost non-struct
> types", don't you?  :-)
> 
> I wouldn't mind seeing kvm_types.h, rwlock_types.h

> and spinlock_types.h

*cough*

You may want to run spinlock_types.h through preprocessor and see how
much garbage it will produce.

> merged into types.h, personally.  They're all pretty fundamental kernel
> kind of types.

Also we care about compilation speed.

> It's a matter of taste, and I'm not particularly fussed one way or the other.
> 
> mm_types.h is complex and full of mm-specific information, so keeping
> it separate makes sense to me.
> 
> I just object to the unnecessary creation of tiny files like this.

Me too. Also jumping over one file to understand what's going on is
better than jumping over multiple files.

> Which is how we ended up with atomic_t and atomic64_t in there in the
> first place :-)

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Arnd Bergmann]

On Friday 02 July 2010 22:48:17 Matthew Wilcox wrote:
> I wouldn't mind seeing kvm_types.h, rwlock_types.h and spinlock_types.h
> merged into types.h, personally.  They're all pretty fundamental kernel
> kind of types.  It's a matter of taste, and I'm not particularly fussed
> one way or the other.
> 
> mm_types.h is complex and full of mm-specific information, so keeping
> it separate makes sense to me.
> 
> I just object to the unnecessary creation of tiny files like this.
> Which is how we ended up with atomic_t and atomic64_t in there in the
> first place :-)

Ah, I didn't notice you had moved the atomic types in there. I agree that
the list types are in the same general category and it makes sense
to treat them the same way.

For rwlock_types.h and spinlock_types.h, I think including them in types.h
would really cause too much other crap to be pulled in through lockdep
and other things we might need in there in the future, which would in turn
cause the same problems with types.h that Chris is trying to avoid
in the first place by moving stuff out of list.h.

	Arnd

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Chris Metcalf]

On 7/3/2010 5:00 AM, Arnd Bergmann wrote:
> On Friday 02 July 2010 22:48:17 Matthew Wilcox wrote:
>   
>> I wouldn't mind seeing kvm_types.h, rwlock_types.h and spinlock_types.h
>> merged into types.h, personally.  They're all pretty fundamental kernel
>> kind of types.  It's a matter of taste, and I'm not particularly fussed
>> one way or the other.
>>
>> mm_types.h is complex and full of mm-specific information, so keeping
>> it separate makes sense to me.
>>
>> I just object to the unnecessary creation of tiny files like this.
>> Which is how we ended up with atomic_t and atomic64_t in there in the
>> first place :-)
>>     
> Ah, I didn't notice you had moved the atomic types in there. I agree that
> the list types are in the same general category and it makes sense
> to treat them the same way.
>
> For rwlock_types.h and spinlock_types.h, I think including them in types.h
> would really cause too much other crap to be pulled in through lockdep
> and other things we might need in there in the future, which would in turn
> cause the same problems with types.h that Chris is trying to avoid
> in the first place by moving stuff out of list.h.
>   

Sounds like we have a consensus on moving the list_head, hlist_head, and
hlist_node types to <linux/types.h>.  I assume everyone is agreed that
initializers, etc., should stay in <linux/list.h>.  I will send out a
revised git patch on Sunday.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH] Break out types from <linux/list.h> to <linux/list_types.h>.

[Matthew Wilcox]

On Sat, Jul 03, 2010 at 09:47:58PM -0400, Chris Metcalf wrote:
> Sounds like we have a consensus on moving the list_head, hlist_head, and
> hlist_node types to <linux/types.h>.  I assume everyone is agreed that
> initializers, etc., should stay in <linux/list.h>.  I will send out a
> revised git patch on Sunday.

Yes, I think that's reasonable.  By way of comparison, ATOMIC_INIT,
etc. stayed in atomic.h.  It makes sense -- you only need the types
for structure definitions, you don't need the initialisers.

I suppose if you have a header which defines an initialiser like sched.h
does, you'll want the list.h initialisers, but anything that complex
should be able to handle including the full list.h header.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."