From: Andrew Morton <akpm@linux-foundation.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: David Rientjes <rientjes@google.com>,
Adrian Bunk <bunk@kernel.org>,
linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
Robert Reif <reif@earthlink.net>
Subject: Re: [patch] sparc: fix build
Date: Wed, 13 Feb 2008 03:08:09 +0000 [thread overview]
Message-ID: <20080212190809.75dbf52e.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080213025739.GO27894@ZenIV.linux.org.uk>
On Wed, 13 Feb 2008 02:57:39 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> On Tue, Feb 12, 2008 at 06:46:54PM -0800, Andrew Morton wrote:
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -20,9 +20,6 @@
> > > #ifndef _LINUX_MEMCONTROL_H
> > > #define _LINUX_MEMCONTROL_H
> > >
> > > -#include <linux/rcupdate.h>
> > > -#include <linux/mm.h>
> > > -
> > > struct mem_cgroup;
> > > struct page_cgroup;
> > > struct page;
> >
> > This really should have been in a separate patch and extensively tested.
> >
> > Have we checked that every file which directly or indirectly includes
> > memcontrol.h does not have an requirement for rcupdate.h and mm.h, where
> > that requirement was satisfied only via this nested inclusion? For all
> > architectures and for all config selections? Think not.
> >
> > Sadly, removal of nested includes is a *big* deal, and it takes quite a lot
> > of time to get it all shaken down.
> >
> > If we can confirm that all files (.c and .h) which include memcontrol.h
> > also directly include rcupdate.h and mm.h then we're _probably_ ok (modulo
> > ordering issues).
> >
> > Otherwise we should perhaps be taking a second look at how to fix the sparc
> > problem.
>
> I've run allmodconfig builds on a bunch of target, FWIW (essentially the
> same patch). Note that these includes are recent addition caused by
> added inline function that had since then become a define. So while I
> agree with your comments in general, in _this_ case it's pretty safe.
OK, thanks, that increases the comfort level,
> Commit that had done it is 3062fc67dad01b1d2a15d58c709eff946389eca4
> and switch to #define is 60c12b1202a60eabb1c61317e5d2678fcea9893f (BTW,
> that warranted mentioning in changelog of the latter).
I just copied-and-pasted your email ;)
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: David Rientjes <rientjes@google.com>,
Adrian Bunk <bunk@kernel.org>,
linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
Robert Reif <reif@earthlink.net>
Subject: Re: [patch] sparc: fix build
Date: Tue, 12 Feb 2008 19:08:09 -0800 [thread overview]
Message-ID: <20080212190809.75dbf52e.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080213025739.GO27894@ZenIV.linux.org.uk>
On Wed, 13 Feb 2008 02:57:39 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> On Tue, Feb 12, 2008 at 06:46:54PM -0800, Andrew Morton wrote:
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -20,9 +20,6 @@
> > > #ifndef _LINUX_MEMCONTROL_H
> > > #define _LINUX_MEMCONTROL_H
> > >
> > > -#include <linux/rcupdate.h>
> > > -#include <linux/mm.h>
> > > -
> > > struct mem_cgroup;
> > > struct page_cgroup;
> > > struct page;
> >
> > This really should have been in a separate patch and extensively tested.
> >
> > Have we checked that every file which directly or indirectly includes
> > memcontrol.h does not have an requirement for rcupdate.h and mm.h, where
> > that requirement was satisfied only via this nested inclusion? For all
> > architectures and for all config selections? Think not.
> >
> > Sadly, removal of nested includes is a *big* deal, and it takes quite a lot
> > of time to get it all shaken down.
> >
> > If we can confirm that all files (.c and .h) which include memcontrol.h
> > also directly include rcupdate.h and mm.h then we're _probably_ ok (modulo
> > ordering issues).
> >
> > Otherwise we should perhaps be taking a second look at how to fix the sparc
> > problem.
>
> I've run allmodconfig builds on a bunch of target, FWIW (essentially the
> same patch). Note that these includes are recent addition caused by
> added inline function that had since then become a define. So while I
> agree with your comments in general, in _this_ case it's pretty safe.
OK, thanks, that increases the comfort level,
> Commit that had done it is 3062fc67dad01b1d2a15d58c709eff946389eca4
> and switch to #define is 60c12b1202a60eabb1c61317e5d2678fcea9893f (BTW,
> that warranted mentioning in changelog of the latter).
I just copied-and-pasted your email ;)
next prev parent reply other threads:[~2008-02-13 3:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-07 23:12 sparc compile error Adrian Bunk
2008-02-07 23:12 ` Adrian Bunk
2008-02-07 23:38 ` David Rientjes
2008-02-07 23:38 ` David Rientjes
2008-02-10 15:33 ` Martin Habets
2008-02-10 15:33 ` Martin Habets
2008-02-10 19:19 ` David Rientjes
2008-02-10 19:19 ` David Rientjes
2008-02-10 21:44 ` Robert Reif
2008-02-10 21:44 ` Robert Reif
2008-02-10 23:30 ` [patch] sparc: fix build David Rientjes
2008-02-10 23:30 ` David Rientjes
2008-02-11 9:37 ` Balbir Singh
2008-02-11 9:49 ` Balbir Singh
2008-02-13 2:46 ` Andrew Morton
2008-02-13 2:46 ` Andrew Morton
2008-02-13 2:57 ` Al Viro
2008-02-13 3:08 ` Andrew Morton [this message]
2008-02-13 3:08 ` Andrew Morton
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=20080212190809.75dbf52e.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=bunk@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=reif@earthlink.net \
--cc=rientjes@google.com \
--cc=sparclinux@vger.kernel.org \
--cc=viro@ZenIV.linux.org.uk \
/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.