public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: "Kaywinnit L. Frye" <kaywinnit.lee.frye.2497@gmail.com>,
	linux-kernel@vger.kernel.org,
	Michal Nazarewicz <m.nazarewicz@samsung.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Evgeny Kuznetsov <EXT-Eugeny.Kuznetsov@nokia.com>
Subject: Re: [PATCH] wait: include linux/sched.h
Date: Tue, 10 May 2011 13:42:17 +0200	[thread overview]
Message-ID: <20110510114217.GA24786@elte.hu> (raw)
In-Reply-To: <1305025620.2914.55.camel@laptop>


* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Tue, 2011-05-10 at 12:36 +0200, Ingo Molnar wrote:
> > 
> > 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. 
> 
> I very well understand the whole header mess, and in particular last time I 
> looked at the sched.h vs wait.h thing simply removing the non sched.c related 
> things was enough to resolve this particular problem.

No!

The wait.h problem is this: why do we want to include sched.h in wait.h?

We do not want it:

 - the waitqueue definitions do not need it
 - the main waitqueue accessor methods do not need it

types using waitqueues should not need to include sched.h as well - directly or 
indirectly!

Why does wait.h 'need' sched.h to work standalone right: because a few 
convenience helper functions in wait.h use scheduler functionality and 
scheduler constants.

As a result of this mess much of wait.h is written in hard to read 1970's tech: 
in the C preprocessor macro language.

Note that even *those* helpers do not really need to know the layout of struct 
task_struct (about which most of sched.h's complexity is): they only need to 
know the task state bits and a few scheduler API methods related to the concept 
of scheduling/waiting.

And note in that context it very much makes sense to have wait_types.h and 
wait_api.h (wait.h): all the derivative types that use waitqueues do not need 
all the helper functions like wait_event() and do not need to include scheduler 
lowlevel context switching details ... Only .c files that *use* waitqueues need 
to include the full kit.

But even the first step of purifying the headers will be a big step forward.

> I really don't like the foo_type.h headers much and think we should only use 
> them as absolute last resort solutions.

I understood your point as cleaning up sched.h to be able to include sched.h in 
wait.h and with that solution i disagree violently: sched.h is about struct 
task_struct and wait.h does not need that - nor do the dozens of other types 
need it that use waitqueues.

We can move all the task definitions and APIs from sched.h into wait.h then yes 
that's the first good first step toward purer structure for both sched.h and 
wait.h.

We can also move signal bits into a separate header, to streamline sched.h some 
more - but sched.h will always be a 'container' kind of super-header, by its 
very nature.

Or if you want some other structure for it all then please outine that 
structure. We could certainly move struct task_struct definition into a 
separate task.h and keep all context switch related definitions in sched.h.

Then task.h will need to be included in tons of code that dereferences 
'current' directly. So for pragmatic reasons it's probably better to keep 
struct task_struct in sched.h. But we can do the more complex change as well, 
as long as there's a plan behind it and as long as there's someone doing it ;-)

> I might have mis-read your email, but to me your:
> 
>   "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.."
> 
> Seems to suggest to begin with that split, instead of starting with sane
> cleanups like moving the signal and process (groups of tasks) bits out
> into their own headers.

Yeah. Either direction is fine to me and moving out unrelated stuff sure is the 
bulk of the work.

Thanks,

	Ingo

      reply	other threads:[~2011-05-10 11:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-02 21:34 [PATCH] wait: include linux/sched.h Kaywinnit L. Frye
2011-05-06  7:54 ` Ingo Molnar
2011-05-10  8:11   ` Peter Zijlstra
2011-05-10 10:36     ` Ingo Molnar
2011-05-10 11:07       ` Peter Zijlstra
2011-05-10 11:42         ` Ingo Molnar [this message]

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=20110510114217.GA24786@elte.hu \
    --to=mingo@elte.hu \
    --cc=EXT-Eugeny.Kuznetsov@nokia.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=gregkh@suse.de \
    --cc=kaywinnit.lee.frye.2497@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.nazarewicz@samsung.com \
    /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