* [PATCH v2 0/1] Fix linux/wait.h header file
@ 2011-02-21 14:38 David Cohen
2011-02-21 14:38 ` [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h David Cohen
2011-02-21 16:33 ` [PATCH v2 0/1] Fix linux/wait.h header file Randy Dunlap
0 siblings, 2 replies; 15+ messages in thread
From: David Cohen @ 2011-02-21 14:38 UTC (permalink / raw)
To: linux-kernel; +Cc: mingo, peterz, linux-omap, linux-media, 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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 14:38 [PATCH v2 0/1] Fix linux/wait.h header file David Cohen @ 2011-02-21 14:38 ` David Cohen 2011-02-21 15:54 ` Peter Zijlstra 2011-02-21 16:33 ` [PATCH v2 0/1] Fix linux/wait.h header file Randy Dunlap 1 sibling, 1 reply; 15+ messages in thread From: David Cohen @ 2011-02-21 14:38 UTC (permalink / raw) To: linux-kernel; +Cc: mingo, peterz, linux-omap, linux-media, 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 14:38 ` [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h David Cohen @ 2011-02-21 15:54 ` Peter Zijlstra 2011-02-21 16:03 ` David Cohen 0 siblings, 1 reply; 15+ messages in thread From: Peter Zijlstra @ 2011-02-21 15:54 UTC (permalink / raw) To: David Cohen; +Cc: linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 15:54 ` Peter Zijlstra @ 2011-02-21 16:03 ` David Cohen 2011-02-21 16:20 ` Peter Zijlstra 0 siblings, 1 reply; 15+ messages in thread From: David Cohen @ 2011-02-21 16:03 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan 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 > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:03 ` David Cohen @ 2011-02-21 16:20 ` Peter Zijlstra 2011-02-21 16:29 ` Felipe Balbi 2011-02-21 17:21 ` Oleg Nesterov 0 siblings, 2 replies; 15+ messages in thread From: Peter Zijlstra @ 2011-02-21 16:20 UTC (permalink / raw) To: David Cohen Cc: linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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.. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:20 ` Peter Zijlstra @ 2011-02-21 16:29 ` Felipe Balbi 2011-02-21 16:43 ` Peter Zijlstra 2011-02-21 17:21 ` Oleg Nesterov 1 sibling, 1 reply; 15+ messages in thread From: Felipe Balbi @ 2011-02-21 16:29 UTC (permalink / raw) To: Peter Zijlstra Cc: David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:29 ` Felipe Balbi @ 2011-02-21 16:43 ` Peter Zijlstra 2011-02-21 16:54 ` Felipe Balbi 0 siblings, 1 reply; 15+ messages in thread From: Peter Zijlstra @ 2011-02-21 16:43 UTC (permalink / raw) To: balbi Cc: David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:43 ` Peter Zijlstra @ 2011-02-21 16:54 ` Felipe Balbi 2011-02-21 17:06 ` Peter Zijlstra 0 siblings, 1 reply; 15+ messages in thread From: Felipe Balbi @ 2011-02-21 16:54 UTC (permalink / raw) To: Peter Zijlstra Cc: balbi, David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:54 ` Felipe Balbi @ 2011-02-21 17:06 ` Peter Zijlstra 2011-02-21 17:18 ` Felipe Balbi 2011-02-21 19:21 ` Alexey Dobriyan 0 siblings, 2 replies; 15+ messages in thread From: Peter Zijlstra @ 2011-02-21 17:06 UTC (permalink / raw) To: balbi Cc: David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 17:06 ` Peter Zijlstra @ 2011-02-21 17:18 ` Felipe Balbi 2011-02-21 19:21 ` Alexey Dobriyan 1 sibling, 0 replies; 15+ messages in thread From: Felipe Balbi @ 2011-02-21 17:18 UTC (permalink / raw) To: Peter Zijlstra Cc: balbi, David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan, Oleg Nesterov 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 17:06 ` Peter Zijlstra 2011-02-21 17:18 ` Felipe Balbi @ 2011-02-21 19:21 ` Alexey Dobriyan 1 sibling, 0 replies; 15+ messages in thread From: Alexey Dobriyan @ 2011-02-21 19:21 UTC (permalink / raw) To: Peter Zijlstra Cc: balbi, David Cohen, linux-kernel, mingo, linux-omap, linux-media, Oleg Nesterov 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 16:20 ` Peter Zijlstra 2011-02-21 16:29 ` Felipe Balbi @ 2011-02-21 17:21 ` Oleg Nesterov 2011-02-21 17:27 ` Oleg Nesterov 1 sibling, 1 reply; 15+ messages in thread From: Oleg Nesterov @ 2011-02-21 17:21 UTC (permalink / raw) To: Peter Zijlstra Cc: David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 17:21 ` Oleg Nesterov @ 2011-02-21 17:27 ` Oleg Nesterov 2011-02-22 15:38 ` David Cohen 0 siblings, 1 reply; 15+ messages in thread From: Oleg Nesterov @ 2011-02-21 17:27 UTC (permalink / raw) To: Peter Zijlstra Cc: David Cohen, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h 2011-02-21 17:27 ` Oleg Nesterov @ 2011-02-22 15:38 ` David Cohen 0 siblings, 0 replies; 15+ messages in thread From: David Cohen @ 2011-02-22 15:38 UTC (permalink / raw) To: Oleg Nesterov Cc: Peter Zijlstra, linux-kernel, mingo, linux-omap, linux-media, Alexey Dobriyan 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. > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/1] Fix linux/wait.h header file 2011-02-21 14:38 [PATCH v2 0/1] Fix linux/wait.h header file David Cohen 2011-02-21 14:38 ` [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h David Cohen @ 2011-02-21 16:33 ` Randy Dunlap 1 sibling, 0 replies; 15+ messages in thread From: Randy Dunlap @ 2011-02-21 16:33 UTC (permalink / raw) To: David Cohen; +Cc: linux-kernel, mingo, peterz, linux-omap, linux-media 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 *** ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-02-22 15:38 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-21 14:38 [PATCH v2 0/1] Fix linux/wait.h header file David Cohen 2011-02-21 14:38 ` [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h David Cohen 2011-02-21 15:54 ` Peter Zijlstra 2011-02-21 16:03 ` David Cohen 2011-02-21 16:20 ` Peter Zijlstra 2011-02-21 16:29 ` Felipe Balbi 2011-02-21 16:43 ` Peter Zijlstra 2011-02-21 16:54 ` Felipe Balbi 2011-02-21 17:06 ` Peter Zijlstra 2011-02-21 17:18 ` Felipe Balbi 2011-02-21 19:21 ` Alexey Dobriyan 2011-02-21 17:21 ` Oleg Nesterov 2011-02-21 17:27 ` Oleg Nesterov 2011-02-22 15:38 ` David Cohen 2011-02-21 16:33 ` [PATCH v2 0/1] Fix linux/wait.h header file Randy Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox