[PATCH v2 0/1] Fix linux/wait.h header file

[PATCH v2 0/1] Fix linux/wait.h header file

[David Cohen]

Hi,

OMAP2 camera driver compilation is broken due to problems on linux/wait.h header
file:

drivers/media/video/omap24xxcam.c: In function 'omap24xxcam_vbq_complete':
drivers/media/video/omap24xxcam.c:414: error: 'TASK_NORMAL' undeclared (first use in this function)
drivers/media/video/omap24xxcam.c:414: error: (Each undeclared identifier is reported only once
drivers/media/video/omap24xxcam.c:414: error: for each function it appears in.)
make[3]: *** [drivers/media/video/omap24xxcam.o] Error 1
make[2]: *** [drivers/media/video] Error 2
make[1]: *** [drivers/media] Error 2
make: *** [drivers] Error 2

This file defines macros wake_up*() which use TASK_* defined on linux/sched.h.
But sched.h cannot be included on wait.h due to a circular dependency between
both files. This patch fixes such compilation and the circular dependency
problem.

Br,

David
---

David Cohen (1):
  headers: fix circular dependency between linux/sched.h and
    linux/wait.h

 include/linux/sched.h      |   58 +-----------------------------------------
 include/linux/task_state.h |   61 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/wait.h       |    1 +
 3 files changed, 63 insertions(+), 57 deletions(-)
 create mode 100644 include/linux/task_state.h

-- 
1.7.2.3

[PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[David Cohen]

Currently sched.h and wait.h have circular dependency between both.
wait.h defines macros wake_up*() which use macros TASK_* defined by
sched.h. But as sched.h indirectly includes wait.h, such wait.h header
file can't include sched.h too. The side effect is when some file
includes wait.h and tries to use its wake_up*() macros, it's necessary
to include sched.h also.
This patch moves all TASK_* macros from linux/sched.h to a new header
file linux/task_state.h. This way, both sched.h and wait.h can include
task_state.h and fix the circular dependency. No need to include sched.h
anymore when wake_up*() macros are used.

Signed-off-by: David Cohen <dacohen@gmail.com>
---
 include/linux/sched.h      |   58 +-----------------------------------------
 include/linux/task_state.h |   61 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/wait.h       |    1 +
 3 files changed, 63 insertions(+), 57 deletions(-)
 create mode 100644 include/linux/task_state.h

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..a75b5ba 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -90,6 +90,7 @@ struct sched_param {
 #include <linux/task_io_accounting.h>
 #include <linux/latencytop.h>
 #include <linux/cred.h>
+#include <linux/task_state.h>
 
 #include <asm/processor.h>
 
@@ -169,63 +170,6 @@ print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 #endif
 
 /*
- * Task state bitmask. NOTE! These bits are also
- * encoded in fs/proc/array.c: get_task_state().
- *
- * We have two separate sets of flags: task->state
- * is about runnability, while task->exit_state are
- * about the task exiting. Confusing, but this way
- * modifying one set can't modify the other one by
- * mistake.
- */
-#define TASK_RUNNING		0
-#define TASK_INTERRUPTIBLE	1
-#define TASK_UNINTERRUPTIBLE	2
-#define __TASK_STOPPED		4
-#define __TASK_TRACED		8
-/* in tsk->exit_state */
-#define EXIT_ZOMBIE		16
-#define EXIT_DEAD		32
-/* in tsk->state again */
-#define TASK_DEAD		64
-#define TASK_WAKEKILL		128
-#define TASK_WAKING		256
-#define TASK_STATE_MAX		512
-
-#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKW"
-
-extern char ___assert_task_state[1 - 2*!!(
-		sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
-
-/* Convenience macros for the sake of set_task_state */
-#define TASK_KILLABLE		(TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)
-#define TASK_STOPPED		(TASK_WAKEKILL | __TASK_STOPPED)
-#define TASK_TRACED		(TASK_WAKEKILL | __TASK_TRACED)
-
-/* Convenience macros for the sake of wake_up */
-#define TASK_NORMAL		(TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE)
-#define TASK_ALL		(TASK_NORMAL | __TASK_STOPPED | __TASK_TRACED)
-
-/* get_task_state() */
-#define TASK_REPORT		(TASK_RUNNING | TASK_INTERRUPTIBLE | \
-				 TASK_UNINTERRUPTIBLE | __TASK_STOPPED | \
-				 __TASK_TRACED)
-
-#define task_is_traced(task)	((task->state & __TASK_TRACED) != 0)
-#define task_is_stopped(task)	((task->state & __TASK_STOPPED) != 0)
-#define task_is_dead(task)	((task)->exit_state != 0)
-#define task_is_stopped_or_traced(task)	\
-			((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
-#define task_contributes_to_load(task)	\
-				((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
-				 (task->flags & PF_FREEZING) == 0)
-
-#define __set_task_state(tsk, state_value)		\
-	do { (tsk)->state = (state_value); } while (0)
-#define set_task_state(tsk, state_value)		\
-	set_mb((tsk)->state, (state_value))
-
-/*
  * set_current_state() includes a barrier so that the write of current->state
  * is correctly serialised wrt the caller's subsequent test of whether to
  * actually sleep:
diff --git a/include/linux/task_state.h b/include/linux/task_state.h
new file mode 100644
index 0000000..36a8db8
--- /dev/null
+++ b/include/linux/task_state.h
@@ -0,0 +1,61 @@
+#ifndef _LINUX_TASK_H
+#define _LINUX_TASK_H
+
+/*
+ * Task state bitmask. NOTE! These bits are also
+ * encoded in fs/proc/array.c: get_task_state().
+ *
+ * We have two separate sets of flags: task->state
+ * is about runnability, while task->exit_state are
+ * about the task exiting. Confusing, but this way
+ * modifying one set can't modify the other one by
+ * mistake.
+ */
+#define TASK_RUNNING		0
+#define TASK_INTERRUPTIBLE	1
+#define TASK_UNINTERRUPTIBLE	2
+#define __TASK_STOPPED		4
+#define __TASK_TRACED		8
+/* in tsk->exit_state */
+#define EXIT_ZOMBIE		16
+#define EXIT_DEAD		32
+/* in tsk->state again */
+#define TASK_DEAD		64
+#define TASK_WAKEKILL		128
+#define TASK_WAKING		256
+#define TASK_STATE_MAX		512
+
+#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKW"
+
+extern char ___assert_task_state[1 - 2*!!(
+		sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
+
+/* Convenience macros for the sake of set_task_state */
+#define TASK_KILLABLE		(TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)
+#define TASK_STOPPED		(TASK_WAKEKILL | __TASK_STOPPED)
+#define TASK_TRACED		(TASK_WAKEKILL | __TASK_TRACED)
+
+/* Convenience macros for the sake of wake_up */
+#define TASK_NORMAL		(TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE)
+#define TASK_ALL		(TASK_NORMAL | __TASK_STOPPED | __TASK_TRACED)
+
+/* get_task_state() */
+#define TASK_REPORT		(TASK_RUNNING | TASK_INTERRUPTIBLE | \
+				 TASK_UNINTERRUPTIBLE | __TASK_STOPPED | \
+				 __TASK_TRACED)
+
+#define task_is_traced(task)	((task->state & __TASK_TRACED) != 0)
+#define task_is_stopped(task)	((task->state & __TASK_STOPPED) != 0)
+#define task_is_dead(task)	((task)->exit_state != 0)
+#define task_is_stopped_or_traced(task)	\
+			((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
+#define task_contributes_to_load(task)	\
+				((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
+				 (task->flags & PF_FREEZING) == 0)
+
+#define __set_task_state(tsk, state_value)		\
+	do { (tsk)->state = (state_value); } while (0)
+#define set_task_state(tsk, state_value)		\
+	set_mb((tsk)->state, (state_value))
+
+#endif
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 3efc9f3..163c2b8 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/task_state.h>
 #include <asm/system.h>
 #include <asm/current.h>
 
-- 
1.7.2.3

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Peter Zijlstra]

On Mon, 2011-02-21 at 16:38 +0200, David Cohen wrote:
> Currently sched.h and wait.h have circular dependency between both.
> wait.h defines macros wake_up*() which use macros TASK_* defined by
> sched.h. But as sched.h indirectly includes wait.h, such wait.h header
> file can't include sched.h too. The side effect is when some file
> includes wait.h and tries to use its wake_up*() macros, it's necessary
> to include sched.h also.
> This patch moves all TASK_* macros from linux/sched.h to a new header
> file linux/task_state.h. This way, both sched.h and wait.h can include
> task_state.h and fix the circular dependency. No need to include sched.h
> anymore when wake_up*() macros are used. 

I think Alexey already told you what you done wrong.

Also, I really don't like the task_state.h header, it assumes a lot of
things it doesn't include itself and only works because its using macros
and not inlines at it probably should.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[David Cohen]

On Mon, Feb 21, 2011 at 5:54 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, 2011-02-21 at 16:38 +0200, David Cohen wrote:
>> Currently sched.h and wait.h have circular dependency between both.
>> wait.h defines macros wake_up*() which use macros TASK_* defined by
>> sched.h. But as sched.h indirectly includes wait.h, such wait.h header
>> file can't include sched.h too. The side effect is when some file
>> includes wait.h and tries to use its wake_up*() macros, it's necessary
>> to include sched.h also.
>> This patch moves all TASK_* macros from linux/sched.h to a new header
>> file linux/task_state.h. This way, both sched.h and wait.h can include
>> task_state.h and fix the circular dependency. No need to include sched.h
>> anymore when wake_up*() macros are used.
>
> I think Alexey already told you what you done wrong.
>
> Also, I really don't like the task_state.h header, it assumes a lot of
> things it doesn't include itself and only works because its using macros
> and not inlines at it probably should.

Like wait.h I'd say. The main issue is wait.h uses TASK_* macros but
cannot properly include sched.h as it would create a circular
dependency. So a file including wait.h is able to compile because the
dependency of sched.h relies on wake_up*() macros and it's not always
used.
We can still drop everything else from task_state.h but the TASK_*
macros and then the problem you just pointed out won't exist anymore.
What do you think about it?

Br,

David

>
>
>

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Peter Zijlstra]

On Mon, 2011-02-21 at 18:03 +0200, David Cohen wrote:
> On Mon, Feb 21, 2011 at 5:54 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, 2011-02-21 at 16:38 +0200, David Cohen wrote:
> >> Currently sched.h and wait.h have circular dependency between both.
> >> wait.h defines macros wake_up*() which use macros TASK_* defined by
> >> sched.h. But as sched.h indirectly includes wait.h, such wait.h header
> >> file can't include sched.h too. The side effect is when some file
> >> includes wait.h and tries to use its wake_up*() macros, it's necessary
> >> to include sched.h also.
> >> This patch moves all TASK_* macros from linux/sched.h to a new header
> >> file linux/task_state.h. This way, both sched.h and wait.h can include
> >> task_state.h and fix the circular dependency. No need to include sched.h
> >> anymore when wake_up*() macros are used.
> >
> > I think Alexey already told you what you done wrong.
> >
> > Also, I really don't like the task_state.h header, it assumes a lot of
> > things it doesn't include itself and only works because its using macros
> > and not inlines at it probably should.
> 
> Like wait.h I'd say. The main issue is wait.h uses TASK_* macros but
> cannot properly include sched.h as it would create a circular
> dependency. So a file including wait.h is able to compile because the
> dependency of sched.h relies on wake_up*() macros and it's not always
> used.
> We can still drop everything else from task_state.h but the TASK_*
> macros and then the problem you just pointed out won't exist anymore.
> What do you think about it?

I'd much rather see a real cleanup.. eg. remove the need for sched.h to
include wait.h.

afaict its needed because struct signal_struct and struct sighand_struct
include a wait_queue_head_t. The inclusion seems to come through
completion.h, but afaict we don't actually need to include completion.h
because all we have is a pointer to a completion, which is perfectly
fine with an incomplete type.

This all would suggest we move the signal bits into their own header
(include/linux/signal.h already exists and seems inviting).

And then make sched.c include signal.h and completion.h.

But then, there might be a 'good' reason these signal bits live in
sched.h and not on their own, but I wouldn't know..

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Felipe Balbi]

Hi,

On Mon, Feb 21, 2011 at 05:20:45PM +0100, Peter Zijlstra wrote:
> > > I think Alexey already told you what you done wrong.
> > >
> > > Also, I really don't like the task_state.h header, it assumes a lot of
> > > things it doesn't include itself and only works because its using macros
> > > and not inlines at it probably should.
> > 
> > Like wait.h I'd say. The main issue is wait.h uses TASK_* macros but
> > cannot properly include sched.h as it would create a circular
> > dependency. So a file including wait.h is able to compile because the
> > dependency of sched.h relies on wake_up*() macros and it's not always
> > used.
> > We can still drop everything else from task_state.h but the TASK_*
> > macros and then the problem you just pointed out won't exist anymore.
> > What do you think about it?
> 
> I'd much rather see a real cleanup.. eg. remove the need for sched.h to
> include wait.h.

isn't that exactly what he's trying to achieve ? Moving TASK_* to its
own header is one approach, what other approach do you suggest ?

> afaict its needed because struct signal_struct and struct sighand_struct
> include a wait_queue_head_t. The inclusion seems to come through

yes.

> completion.h, but afaict we don't actually need to include completion.h
> because all we have is a pointer to a completion, which is perfectly
> fine with an incomplete type.

so maybe just dropping completion.h from sched.h would do it.

> This all would suggest we move the signal bits into their own header
> (include/linux/signal.h already exists and seems inviting).
> 
> And then make sched.c include signal.h and completion.h.

you wouldn't prevent the underlying problem which is the need to include
sched.h whenever you include wait.h and use wake_up*()

-- 
balbi

Re: [PATCH v2 0/1] Fix linux/wait.h header file

[Randy Dunlap]

On Mon, 21 Feb 2011 16:38:50 +0200 David Cohen wrote:

> Hi,
> 
> OMAP2 camera driver compilation is broken due to problems on linux/wait.h header
> file:
> 
> drivers/media/video/omap24xxcam.c: In function 'omap24xxcam_vbq_complete':
> drivers/media/video/omap24xxcam.c:414: error: 'TASK_NORMAL' undeclared (first use in this function)
> drivers/media/video/omap24xxcam.c:414: error: (Each undeclared identifier is reported only once
> drivers/media/video/omap24xxcam.c:414: error: for each function it appears in.)
> make[3]: *** [drivers/media/video/omap24xxcam.o] Error 1
> make[2]: *** [drivers/media/video] Error 2
> make[1]: *** [drivers/media] Error 2
> make: *** [drivers] Error 2
> 
> This file defines macros wake_up*() which use TASK_* defined on linux/sched.h.
> But sched.h cannot be included on wait.h due to a circular dependency between
> both files. This patch fixes such compilation and the circular dependency
> problem.
> 
> Br,
> 
> David
> ---
> 
> David Cohen (1):
>   headers: fix circular dependency between linux/sched.h and
>     linux/wait.h
> 
>  include/linux/sched.h      |   58 +-----------------------------------------
>  include/linux/task_state.h |   61 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/wait.h       |    1 +
>  3 files changed, 63 insertions(+), 57 deletions(-)
>  create mode 100644 include/linux/task_state.h
> 
> -- 

and please drop patch 0/1.  All of this info can be included in patch 1/1,
either above the first "---" line (for commit info) or after it (for
background info).

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Peter Zijlstra]

On Mon, 2011-02-21 at 18:29 +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 21, 2011 at 05:20:45PM +0100, Peter Zijlstra wrote:
> > > > I think Alexey already told you what you done wrong.
> > > >
> > > > Also, I really don't like the task_state.h header, it assumes a lot of
> > > > things it doesn't include itself and only works because its using macros
> > > > and not inlines at it probably should.
> > > 
> > > Like wait.h I'd say. The main issue is wait.h uses TASK_* macros but
> > > cannot properly include sched.h as it would create a circular
> > > dependency. So a file including wait.h is able to compile because the
> > > dependency of sched.h relies on wake_up*() macros and it's not always
> > > used.
> > > We can still drop everything else from task_state.h but the TASK_*
> > > macros and then the problem you just pointed out won't exist anymore.
> > > What do you think about it?
> > 
> > I'd much rather see a real cleanup.. eg. remove the need for sched.h to
> > include wait.h.
> 
> isn't that exactly what he's trying to achieve ? Moving TASK_* to its
> own header is one approach, what other approach do you suggest ?

No, he's making a bigger mess, and didn't I just make another
suggestion?

> > afaict its needed because struct signal_struct and struct sighand_struct
> > include a wait_queue_head_t. The inclusion seems to come through
> 
> yes.

Is that a qualified statement that, yes, that is the only inclusion
path?

> > completion.h, but afaict we don't actually need to include completion.h
> > because all we have is a pointer to a completion, which is perfectly
> > fine with an incomplete type.
> 
> so maybe just dropping completion.h from sched.h would do it.

No, that will result in non-compilation due to wait_queue_head_t usage.

> > This all would suggest we move the signal bits into their own header
> > (include/linux/signal.h already exists and seems inviting).
> > 
> > And then make sched.c include signal.h and completion.h.
> 
> you wouldn't prevent the underlying problem which is the need to include
> sched.h whenever you include wait.h and use wake_up*()

If you'd applied your brain for a second before hitting reply you'd have
noticed that at this point you'd (likely) be able to include sched.h
from wait.h. which is the right way about, you need to be able to
schedule in order to build waitqueues.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Felipe Balbi]

Hi,

On Mon, Feb 21, 2011 at 05:43:27PM +0100, Peter Zijlstra wrote:
> > > And then make sched.c include signal.h and completion.h.
> > 
> > you wouldn't prevent the underlying problem which is the need to include
> > sched.h whenever you include wait.h and use wake_up*()
> 
> If you'd applied your brain for a second before hitting reply you'd have
> noticed that at this point you'd (likely) be able to include sched.h
> from wait.h. which is the right way about, you need to be able to
> schedule in order to build waitqueues.

someone's in a good mood today ;-)

What you seem to have missed is that sched.h doesn't include wait.h, it
includes completion.h and completion.h needs wait.h due the
wait_queue_head_t it uses.

If someone finds a cleaner way to drop that need, then I'm all for it as
my original suggestion to the original patch was to include sched.h in
wait.h, but it turned out that it's not possible due to the reasons
already explained.

-- 
balbi

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Peter Zijlstra]

On Mon, 2011-02-21 at 18:54 +0200, Felipe Balbi wrote:

> What you seem to have missed is that sched.h doesn't include wait.h, it
> includes completion.h and completion.h needs wait.h due the
> wait_queue_head_t it uses.

Yeah, so? sched.h doesn't need completion.h, but like with wait.h I'd
argue the other way around, completion.h would want to include sched.h

> If someone finds a cleaner way to drop that need, then I'm all for it as
> my original suggestion to the original patch was to include sched.h in
> wait.h, but it turned out that it's not possible due to the reasons
> already explained.

Feh,. I'm saying the proposed solution stinks and if you want to make
things better you need to work on fixing whatever is in the way of
including sched.h from wait.h.

1) remove the inclusion of completion.h -- easy we can live with an
incomplete type.

2) move the other wait_queue_head_t users (signal_struct sighand_struct)
out of sched.h

3) ...

4) profit!

Just isolating the TASK_state bits isn't going to be enough, wait.h also
wants wake_up goo and schedule*(), therefore either include sched.h from
whatever .c file you're using wait.h bits or do the above cleanup.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Felipe Balbi]

On Mon, Feb 21, 2011 at 06:06:02PM +0100, Peter Zijlstra wrote:
> On Mon, 2011-02-21 at 18:54 +0200, Felipe Balbi wrote:
> 
> > What you seem to have missed is that sched.h doesn't include wait.h, it
> > includes completion.h and completion.h needs wait.h due the
> > wait_queue_head_t it uses.
> 
> Yeah, so? sched.h doesn't need completion.h, but like with wait.h I'd
> argue the other way around, completion.h would want to include sched.h

ok, now I get what you proposed. Still, we could have lived without the
sarcasm, but that's not subject to patching.

-- 
balbi

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Oleg Nesterov]

On 02/21, Peter Zijlstra wrote:
>
> afaict its needed because struct signal_struct and struct sighand_struct
> include a wait_queue_head_t. The inclusion seems to come through
> completion.h, but afaict we don't actually need to include completion.h
> because all we have is a pointer to a completion, which is perfectly
> fine with an incomplete type.
>
> This all would suggest we move the signal bits into their own header
> (include/linux/signal.h already exists and seems inviting).

Agreed, sched.h contatins a lot of garbage, including the signal bits.

As for signal_struct in particular I am not really sure, it is just
misnamed. It is in fact "struct process" or "struct thread_group". But
dequeue_signal/etc should go into signal.h.

The only problem, it is not clear how to test such a change.

Oleg.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Oleg Nesterov]

On 02/21, Oleg Nesterov wrote:
>
> On 02/21, Peter Zijlstra wrote:
> >
> > afaict its needed because struct signal_struct and struct sighand_struct
> > include a wait_queue_head_t. The inclusion seems to come through
> > completion.h, but afaict we don't actually need to include completion.h
> > because all we have is a pointer to a completion, which is perfectly
> > fine with an incomplete type.
> >
> > This all would suggest we move the signal bits into their own header
> > (include/linux/signal.h already exists and seems inviting).
>
> Agreed, sched.h contatins a lot of garbage, including the signal bits.
>
> As for signal_struct in particular I am not really sure, it is just
> misnamed. It is in fact "struct process" or "struct thread_group". But
> dequeue_signal/etc should go into signal.h.
>
> The only problem, it is not clear how to test such a change.

Ah. sched.h includes signal.h, the testing is not the problem.

So, we can (at least) safely move some declarations.

Oleg.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[Alexey Dobriyan]

On Mon, Feb 21, 2011 at 06:06:02PM +0100, Peter Zijlstra wrote:
> 1) remove the inclusion of completion.h -- easy we can live with an
> incomplete type.

ACK

> 2) move the other wait_queue_head_t users (signal_struct sighand_struct)
> out of sched.h
> 
> 3) ...

Compile test! :^)

> 4) profit!
> 
> Just isolating the TASK_state bits isn't going to be enough, wait.h also
> wants wake_up goo and schedule*(), therefore either include sched.h from
> whatever .c file you're using wait.h bits or do the above cleanup.

Speaking of junk in sched.h, I'll send mm_types.h removal next merge window
and maybe cred.h.

Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h

[David Cohen]

On Mon, Feb 21, 2011 at 7:27 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 02/21, Oleg Nesterov wrote:
>>
>> On 02/21, Peter Zijlstra wrote:
>> >
>> > afaict its needed because struct signal_struct and struct sighand_struct
>> > include a wait_queue_head_t. The inclusion seems to come through
>> > completion.h, but afaict we don't actually need to include completion.h
>> > because all we have is a pointer to a completion, which is perfectly
>> > fine with an incomplete type.
>> >
>> > This all would suggest we move the signal bits into their own header
>> > (include/linux/signal.h already exists and seems inviting).
>>
>> Agreed, sched.h contatins a lot of garbage, including the signal bits.
>>
>> As for signal_struct in particular I am not really sure, it is just
>> misnamed. It is in fact "struct process" or "struct thread_group". But
>> dequeue_signal/etc should go into signal.h.
>>
>> The only problem, it is not clear how to test such a change.
>
> Ah. sched.h includes signal.h, the testing is not the problem.

If sched.h includes signal.h and we move wait_queue_head_t users to
signal.h, it means signal.h should include wait.h and then it is a
problem to include sched.h in wait.h.

>
> So, we can (at least) safely move some declarations.

Safely, yes, but it won't solve the issue for TASK_* in wait.h.

Br,

David

>
> Oleg.
>
>