Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
On Mon, Feb 21, 2011 at 7:27 PM, Oleg Nesterov o...@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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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 daco...@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_ZOMBIE16
-#define EXIT_DEAD 32
-/* in tsk-state again */
-#define TASK_DEAD 64
-#define TASK_WAKEKILL 128
-#define TASK_WAKING256
-#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 000..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_ZOMBIE16
+#define EXIT_DEAD 32
+/* in tsk-state again */
+#define TASK_DEAD 64
+#define TASK_WAKEKILL 128
+#define TASK_WAKING256
+#define TASK_STATE_MAX 512
+
+#define TASK_STATE_TO_CHAR_STR RSDTtZXxKW
+
+extern char ___assert_task_state[1 -
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
On Mon, Feb 21, 2011 at 5:54 PM, Peter Zijlstra pet...@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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
On Mon, 2011-02-21 at 18:03 +0200, David Cohen wrote: On Mon, Feb 21, 2011 at 5:54 PM, Peter Zijlstra pet...@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.. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] headers: fix circular dependency between linux/sched.h and linux/wait.h
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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
