Messages in this thread | | | Date | Tue, 10 May 2011 12:36:28 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] wait: include linux/sched.h |
| |
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Fri, 2011-05-06 at 09:54 +0200, Ingo Molnar wrote: > > > > diff --git a/include/linux/wait.h b/include/linux/wait.h > > > index 3efc9f3..667a3d7 100644 > > > --- a/include/linux/wait.h > > > +++ b/include/linux/wait.h > > > @@ -22,6 +22,7 @@ > > > #include <linux/list.h> > > > #include <linux/stddef.h> > > > #include <linux/spinlock.h> > > > +#include <linux/sched.h> > > > #include <asm/system.h> > > > #include <asm/current.h> > > > > I'm quite sure this will break the build all around the place, because sched.h > > itself uses wait.h primitives. > > I'm very sure it will, people keep proposing this. > > > To fix these super-headers like sched.h and to make wait.h self-sufficient the > > right and cleanest approach would be to split data types and primitive accessor > > functions from higher level helper methods (which inevitably mix different > > domains) and put them into two separate files. > > > > We have a few such split files in the kernel tree, spinlock_types.h and > > spinlock_api.h, and this concept works reasonably well. > > > > So here we'd need wait_types.h and wait_api.h and of course sched_types.h and > > sched_api.h. > > > > For simplicity of migration wait_api.h could be wait.h itself and sched_api.h > > could be sched.h itself. > > Argh, please don't. I've been arguing against that for a while now.
Yours is not a very well argued argument i have to say ;-)
> The right thing to do is to clean up sched.h and remove a lot of the > non-scheduler bits in there, like all the process and signal bits.
That, in fact, is the first half of what i suggested - obviously type splitting means first moving unrelated bits out of it.
Lets call it 'purifying' the header.
But, and i talk from first-hand experience here, even if we do that the core problem creating header spaghetti still remains, even if a header file is 'pure': helper/utility inline functions using 'other' types which requires the inclusion of those other types - this quickly explodes.
Lets see an example, say we have two types and two headers:
header-A.h:
type A method::A1 method::A2
header-B.h:
type B method::B1 method::B2
And header-B also includes helper functions, which often have the form of:
helper-method::B1() { type A = method::A1(); type B;
if (B.field) return A;
return NULL; }
Then this 'imports' type A into type B's header file - although type B does not really relate to type A, only the helper method B1 has some (often superficial) connection to it.
And if we have a similar helper method in header-A, making use of header-B, we have inclusion circular dependency. These are typically worked around via ugly #define()s. So we have tons of ugly defines and messy dependencyes.
So if we separate "types + type-specific methods" from "helper functions" (after all the cleanup you suggested as well, i.e. moving unrelated functionality out), we have improved the situation materially: both header-A.h and header-B.h become 'pure' and there's also header-B-helpers.h that can freely mix type-A and type-B methods.
Once that split is done other types can also thus include header-B.h without implicitly including header-A and all of its dependencies.
The end result is that we get a clean hiearchy of types.
Obviously sched.h would include a lot of basic types to begin with, because it is a fundamental 'container' type (struct task_struct) sitting at a top, highlevel node of the type hierarchy - so the initial step you suggest would indeed get us most of the benefits.
Thanks,
Ingo
| |