[rfc,patch] breaking up sched.h

[rfc,patch] breaking up sched.h

[Tim Schmielau]

While it's quite common to do "current->foo", every file doing
so needs to #include <linux/sched.h> for the declaration of
task_struct.
In order to reduce the number of #include <linux/sched.h>, I'd propose to
move task_struct to a separate header file (patch below), and include it
from <asm/current.h>.

Other common causes for unnecessary sched.h includes are

  request_irq()
  free_irq()
  suser()
  fsuser()
  capable()

While the latter three are probably better hosted in
<linux/capability.h>, I'm unsure where to move the former two.
<linux/irq.h> seems quite natural, but it starts with a comment

  /*
   * Please do not include this file in generic code.  There is currently
   * no requirement for any architecture to implement anything held
   * within this file.
   *
   * Thanks. --rmk
   */

so that seems to be no-go. Any alternative suggestions?

Tim


--- linux-2.5.17/include/linux/sched.h	Tue May 21 07:07:31 2002
+++ linux-2.5.17-jc/include/linux/sched.h	Fri May 24 21:53:20 2002
@@ -129,6 +129,7 @@
 #ifdef __KERNEL__
 
 #include <linux/spinlock.h>
+#include <linux/task_struct.h>
 
 /*
  * This serializes "schedule()" and also protects
@@ -246,125 +247,6 @@
 #define INIT_USER (&root_user)
 
 typedef struct prio_array prio_array_t;
-
-struct task_struct {
-	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
-	struct thread_info *thread_info;
-	atomic_t usage;
-	unsigned long flags;	/* per process flags, defined below */
-	unsigned long ptrace;
-
-	int lock_depth;		/* Lock depth */
-
-	int prio, static_prio;
-	list_t run_list;
-	prio_array_t *array;
-
-	unsigned long sleep_avg;
-	unsigned long sleep_timestamp;
-
-	unsigned long policy;
-	unsigned long cpus_allowed;
-	unsigned int time_slice;
-
-	struct list_head tasks;
-
-	struct mm_struct *mm, *active_mm;
-	struct list_head local_pages;
-
-	unsigned int allocation_order, nr_local_pages;
-
-/* task state */
-	struct linux_binfmt *binfmt;
-	int exit_code, exit_signal;
-	int pdeath_signal;  /*  The signal sent when the parent dies  */
-	/* ??? */
-	unsigned long personality;
-	int did_exec:1;
-	pid_t pid;
-	pid_t pgrp;
-	pid_t tty_old_pgrp;
-	pid_t session;
-	pid_t tgid;
-	/* boolean value for session group leader */
-	int leader;
-	/* 
-	 * pointers to (original) parent process, youngest child, younger sibling,
-	 * older sibling, respectively.  (p->father can be replaced with 
-	 * p->parent->pid)
-	 */
-	struct task_struct *real_parent; /* real parent process (when being debugged) */
-	struct task_struct *parent;	/* parent process */
-	struct list_head children;	/* list of my children */
-	struct list_head sibling;	/* linkage in my parent's children list */
-	struct list_head thread_group;
-
-	/* PID hash table linkage. */
-	struct task_struct *pidhash_next;
-	struct task_struct **pidhash_pprev;
-
-	wait_queue_head_t wait_chldexit;	/* for wait4() */
-	struct completion *vfork_done;		/* for vfork() */
-
-	unsigned long rt_priority;
-	unsigned long it_real_value, it_prof_value, it_virt_value;
-	unsigned long it_real_incr, it_prof_incr, it_virt_incr;
-	struct timer_list real_timer;
-	struct tms times;
-	unsigned long start_time;
-	long per_cpu_utime[NR_CPUS], per_cpu_stime[NR_CPUS];
-/* mm fault and swap info: this can arguably be seen as either mm-specific or thread-specific */
-	unsigned long min_flt, maj_flt, nswap, cmin_flt, cmaj_flt, cnswap;
-	int swappable:1;
-/* process credentials */
-	uid_t uid,euid,suid,fsuid;
-	gid_t gid,egid,sgid,fsgid;
-	int ngroups;
-	gid_t	groups[NGROUPS];
-	kernel_cap_t   cap_effective, cap_inheritable, cap_permitted;
-	int keep_capabilities:1;
-	struct user_struct *user;
-/* limits */
-	struct rlimit rlim[RLIM_NLIMITS];
-	unsigned short used_math;
-	char comm[16];
-/* file system info */
-	int link_count, total_link_count;
-	struct tty_struct *tty; /* NULL if no tty */
-	unsigned int locks; /* How many file locks are being held */
-/* ipc stuff */
-	struct sysv_sem sysvsem;
-/* CPU-specific state of this task */
-	struct thread_struct thread;
-/* filesystem information */
-	struct fs_struct *fs;
-/* open file information */
-	struct files_struct *files;
-/* namespace */
-	struct namespace *namespace;
-/* signal handlers */
-	spinlock_t sigmask_lock;	/* Protects signal and blocked */
-	struct signal_struct *sig;
-
-	sigset_t blocked;
-	struct sigpending pending;
-
-	unsigned long sas_ss_sp;
-	size_t sas_ss_size;
-	int (*notifier)(void *priv);
-	void *notifier_data;
-	sigset_t *notifier_mask;
-	
-/* Thread group tracking */
-   	u32 parent_exec_id;
-   	u32 self_exec_id;
-/* Protection of (de-)allocation: mm, files, fs, tty */
-	spinlock_t alloc_lock;
-
-/* journalling filesystem info */
-	void *journal_info;
-	struct dentry *proc_dentry;
-};
 
 extern void __put_task_struct(struct task_struct *tsk);
 #define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)

--- linux-2.5.17/include/linux/task_struct.h	Thu Jan  1 01:00:00 1970
+++ linux-2.5.17-jc/include/linux/task_struct.h	Fri May 24 21:52:03 2002
@@ -0,0 +1,128 @@
+#ifndef _LINUX_TASK_STRUCT_H
+#define _LINUX_TASK_STRUCT_H
+
+#ifdef __KERNEL__
+
+struct task_struct {
+	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
+	struct thread_info *thread_info;
+	atomic_t usage;
+	unsigned long flags;	/* per process flags, defined below */
+	unsigned long ptrace;
+
+	int lock_depth;		/* Lock depth */
+
+	int prio, static_prio;
+	list_t run_list;
+	prio_array_t *array;
+
+	unsigned long sleep_avg;
+	unsigned long sleep_timestamp;
+
+	unsigned long policy;
+	unsigned long cpus_allowed;
+	unsigned int time_slice;
+
+	struct list_head tasks;
+
+	struct mm_struct *mm, *active_mm;
+	struct list_head local_pages;
+
+	unsigned int allocation_order, nr_local_pages;
+
+/* task state */
+	struct linux_binfmt *binfmt;
+	int exit_code, exit_signal;
+	int pdeath_signal;  /*  The signal sent when the parent dies  */
+	/* ??? */
+	unsigned long personality;
+	int did_exec:1;
+	pid_t pid;
+	pid_t pgrp;
+	pid_t tty_old_pgrp;
+	pid_t session;
+	pid_t tgid;
+	/* boolean value for session group leader */
+	int leader;
+	/* 
+	 * pointers to (original) parent process, youngest child, younger sibling,
+	 * older sibling, respectively.  (p->father can be replaced with 
+	 * p->parent->pid)
+	 */
+	struct task_struct *real_parent; /* real parent process (when being debugged) */
+	struct task_struct *parent;	/* parent process */
+	struct list_head children;	/* list of my children */
+	struct list_head sibling;	/* linkage in my parent's children list */
+	struct list_head thread_group;
+
+	/* PID hash table linkage. */
+	struct task_struct *pidhash_next;
+	struct task_struct **pidhash_pprev;
+
+	wait_queue_head_t wait_chldexit;	/* for wait4() */
+	struct completion *vfork_done;		/* for vfork() */
+
+	unsigned long rt_priority;
+	unsigned long it_real_value, it_prof_value, it_virt_value;
+	unsigned long it_real_incr, it_prof_incr, it_virt_incr;
+	struct timer_list real_timer;
+	struct tms times;
+	unsigned long start_time;
+	long per_cpu_utime[NR_CPUS], per_cpu_stime[NR_CPUS];
+/* mm fault and swap info: this can arguably be seen as either mm-specific or thread-specific */
+	unsigned long min_flt, maj_flt, nswap, cmin_flt, cmaj_flt, cnswap;
+	int swappable:1;
+/* process credentials */
+	uid_t uid,euid,suid,fsuid;
+	gid_t gid,egid,sgid,fsgid;
+	int ngroups;
+	gid_t	groups[NGROUPS];
+	kernel_cap_t   cap_effective, cap_inheritable, cap_permitted;
+	int keep_capabilities:1;
+	struct user_struct *user;
+/* limits */
+	struct rlimit rlim[RLIM_NLIMITS];
+	unsigned short used_math;
+	char comm[16];
+/* file system info */
+	int link_count, total_link_count;
+	struct tty_struct *tty; /* NULL if no tty */
+	unsigned int locks; /* How many file locks are being held */
+/* ipc stuff */
+	struct sysv_sem sysvsem;
+/* CPU-specific state of this task */
+	struct thread_struct thread;
+/* filesystem information */
+	struct fs_struct *fs;
+/* open file information */
+	struct files_struct *files;
+/* namespace */
+	struct namespace *namespace;
+/* signal handlers */
+	spinlock_t sigmask_lock;	/* Protects signal and blocked */
+	struct signal_struct *sig;
+
+	sigset_t blocked;
+	struct sigpending pending;
+
+	unsigned long sas_ss_sp;
+	size_t sas_ss_size;
+	int (*notifier)(void *priv);
+	void *notifier_data;
+	sigset_t *notifier_mask;
+	
+/* Thread group tracking */
+   	u32 parent_exec_id;
+   	u32 self_exec_id;
+/* Protection of (de-)allocation: mm, files, fs, tty */
+	spinlock_t alloc_lock;
+
+/* journalling filesystem info */
+	void *journal_info;
+	struct dentry *proc_dentry;
+};
+
+
+#endif /* __KERNEL__ */
+
+#endif

Re: [rfc,patch] breaking up sched.h

[Russell King]

On Fri, May 24, 2002 at 10:34:04PM +0200, Tim Schmielau wrote:
> While the latter three are probably better hosted in
> <linux/capability.h>, I'm unsure where to move the former two.
> <linux/irq.h> seems quite natural, but it starts with a comment
> 
>   /*
>    * Please do not include this file in generic code.  There is currently
>    * no requirement for any architecture to implement anything held
>    * within this file.
>    *
>    * Thanks. --rmk
>    */
> 
> so that seems to be no-go. Any alternative suggestions?

The problem with linux/irq.h is that it makes many assumptions about the
per-architecture irq code.  If anyone needs to include it (in its current
form), then they're accessing architecture private data that may or may
not be present.

Maybe the correct thing would be to move linux/irq.h to linux/hw_irq.h
(so it matches asm/hw_irq.h) and move request_irq() and friends into a
new linux/irq.h

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [rfc,patch] breaking up sched.h

[Tim Schmielau]

[lkml: sorry for double posting by mistake]

On Fri, 24 May 2002, Russell King wrote:

> The problem with linux/irq.h is that it makes many assumptions about the
> per-architecture irq code.  If anyone needs to include it (in its current
> form), then they're accessing architecture private data that may or may
> not be present.
> 
> Maybe the correct thing would be to move linux/irq.h to linux/hw_irq.h
> (so it matches asm/hw_irq.h) and move request_irq() and friends into a
> new linux/irq.h

Thank you.
I will do so in the sched.h cleanup patches that my heavy 
grepping will result in (I hope).

Tim

Re: [rfc,patch] breaking up sched.h

[Daniel Phillips]

On Friday 24 May 2002 22:34, Tim Schmielau wrote:
> While it's quite common to do "current->foo", every file doing
> so needs to #include <linux/sched.h> for the declaration of
> task_struct.
> In order to reduce the number of #include <linux/sched.h>, I'd propose to
> move task_struct to a separate header file (patch below), and include it
> from <asm/current.h>.

Such separation of data vs function declarations is a good thing.  This
results in a source tree which is easier to work with and contains fewer
strange hacks to get around include order problems.  Possibly, the kernel
may end up compiling faster as well, if some of the includes aimed merely
at obtaining the data definitions don't have to pick up the (much
bulkier) operation definitions as well.

This is the same technique I used in my 'early_page' patch set, which 
gave me the ability to rewrite all the address operations in the page.h
and pgtable.h headers as inline functions instead of macros.

-- 
Daniel