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

[PATCH 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      |   61 +-----------------------------------------
 include/linux/task_sched.h |   64 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/wait.h       |    1 +
 3 files changed, 66 insertions(+), 60 deletions(-)
 create mode 100644 include/linux/task_sched.h

[PATCH 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_sched.h. This way, both sched.h and wait.h can include
task_sched.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      |   61 +-----------------------------------------
 include/linux/task_sched.h |   64 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/wait.h       |    1 +
 3 files changed, 66 insertions(+), 60 deletions(-)
 create mode 100644 include/linux/task_sched.h

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..c60dcee 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_sched.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:
@@ -241,9 +185,6 @@ extern char ___assert_task_state[1 - 2*!!(
 #define set_current_state(state_value)		\
 	set_mb(current->state, (state_value))
 
-/* Task command name length */
-#define TASK_COMM_LEN 16
-
 #include <linux/spinlock.h>
 
 /*
diff --git a/include/linux/task_sched.h b/include/linux/task_sched.h
new file mode 100644
index 0000000..3787387
--- /dev/null
+++ b/include/linux/task_sched.h
@@ -0,0 +1,64 @@
+#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))
+
+/* Task command name length */
+#define TASK_COMM_LEN 16
+
+#endif
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 3efc9f3..cccfdd2 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_sched.h>
 #include <asm/system.h>
 #include <asm/current.h>
 
-- 
1.7.1

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

[Alexey Dobriyan]

On Mon, Feb 21, 2011 at 12:20 PM, David Cohen <dacohen@gmail.com> 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_sched.h. This way, both sched.h and wait.h can include
> task_sched.h and fix the circular dependency. No need to include sched.h
> anymore when wake_up*() macros are used.

Just include <linux/sched.h> in your driver.
This include splitting in small pieces is troublesome as well.

Why are you moving TASK_COMM_LEN?

>  include/linux/sched.h      |   61 +-----------------------------------------
>  include/linux/task_sched.h |   64 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/wait.h       |    1 +
>  3 files changed, 66 insertions(+), 60 deletions(-)
>  create mode 100644 include/linux/task_sched.h

> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> +#include <linux/task_sched.h>

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

[David Cohen]

On Mon, Feb 21, 2011 at 1:05 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Mon, Feb 21, 2011 at 12:20 PM, David Cohen <dacohen@gmail.com> 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_sched.h. This way, both sched.h and wait.h can include
>> task_sched.h and fix the circular dependency. No need to include sched.h
>> anymore when wake_up*() macros are used.
>
> Just include <linux/sched.h> in your driver.

Sounds a reasonable solution for me.

> This include splitting in small pieces is troublesome as well.

But I disagree it's troublesome. It's transparent to everyone else.
The only side effect is to not have to include sched.h when using a
macro define on wait.h.

>
> Why are you moving TASK_COMM_LEN?

This one can be moved back to sched.h.

Br,

David

>
>>  include/linux/sched.h      |   61 +-----------------------------------------
>>  include/linux/task_sched.h |   64 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/wait.h       |    1 +
>>  3 files changed, 66 insertions(+), 60 deletions(-)
>>  create mode 100644 include/linux/task_sched.h
>
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> +#include <linux/task_sched.h>
>

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

[Felipe Balbi]

On Mon, Feb 21, 2011 at 01:05:51PM +0200, Alexey Dobriyan wrote:
> On Mon, Feb 21, 2011 at 12:20 PM, David Cohen <dacohen@gmail.com> 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_sched.h. This way, both sched.h and wait.h can include
> > task_sched.h and fix the circular dependency. No need to include sched.h
> > anymore when wake_up*() macros are used.
> 
> Just include <linux/sched.h> in your driver.
> This include splitting in small pieces is troublesome as well.

so, simply to call wake_up*() we need to know everything there is to
know about the scheduler ? I rather have the split done and kill the
circular dependency. What does Mingo and Peter think about this ?

-- 
balbi

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

[Alexey Dobriyan]

On Mon, Feb 21, 2011 at 2:30 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Feb 21, 2011 at 01:05:51PM +0200, Alexey Dobriyan wrote:

>> Just include <linux/sched.h> in your driver.
>> This include splitting in small pieces is troublesome as well.
>
> so, simply to call wake_up*() we need to know everything there is to
> know about the scheduler ?

"know everything" is exaggeration.
You add sched.h, if someone else haven't added it already via other headers.

File is misnamed, BTW. It should be called task-state.h or something.
And, again, TASK_COMM_LEN should be left alone.

> I rather have the split done and kill the circular dependency.

It's not circular for starters.

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

[Felipe Balbi]

Hi,

On Mon, Feb 21, 2011 at 03:51:25PM +0200, Alexey Dobriyan wrote:
> > I rather have the split done and kill the circular dependency.
> 
> It's not circular for starters.

how come ? wait.h depends on sched and sched.h depends on wait.h

-- 
balbi

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

[David Cohen]

On Mon, Feb 21, 2011 at 3:57 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Feb 21, 2011 at 03:51:25PM +0200, Alexey Dobriyan wrote:
>> > I rather have the split done and kill the circular dependency.
>>
>> It's not circular for starters.
>
> how come ? wait.h depends on sched and sched.h depends on wait.h

The tricky thing is wait.h doesn't depend on sched.h, but the file
which uses wake_up*() macro defined on wait.h will depend on sched.h
(what is still bad). wait.h should provide all dependencies to use a
macro it defines. I'll send a new version for this patch following the
comments I got. Let's see how it looks like.

Br,

David

>
> --
> balbi
>

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

[Felipe Balbi]

Hi,

On Mon, Feb 21, 2011 at 04:05:14PM +0200, David Cohen wrote:
> On Mon, Feb 21, 2011 at 3:57 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Mon, Feb 21, 2011 at 03:51:25PM +0200, Alexey Dobriyan wrote:
> >> > I rather have the split done and kill the circular dependency.
> >>
> >> It's not circular for starters.
> >
> > how come ? wait.h depends on sched and sched.h depends on wait.h
> 
> The tricky thing is wait.h doesn't depend on sched.h, but the file
> which uses wake_up*() macro defined on wait.h will depend on sched.h
> (what is still bad). wait.h should provide all dependencies to use a

That's why I say wait.h depends on sched.h because it uses a macro
defined in sched.h

-- 
balbi