Skip to contentSkip to conversation

obscure pid implementation fix (v2)

PATCH In review Kirill Korotaev 3 participants 13 messages Sep 1, 2004

Subsystems (8)
VFS MM core Scheduler Char and misc drivers +4 more subsystems
lore
Share & tools
lore

Kirill Korotaev

author
#0 lore raw

PATCH 1/1 obscure pid implementation fix (v2)

I remade the previous patch against the latest Linus tree, please apply.

This patch fixes strange and obscure pid implementation in current kernels:
- it removes calling of put_task_struct() from detach_pid() under tasklist_lock. This allows to use blocking calls
in security_task_free() hooks (in __put_task_struct()).
- it saves some space = 5*5 ints = 100 bytes in task_struct
- it's smaller and tidy, more straigthforward and doesn't use any knowledge about pids using and assignment.
- it removes pid_links and pid_struct doesn't hold reference counters on task_struct. instead, new pid_structs and linked altogether and only one of them is inserted in hash_list.

Signed-off-by Kirill Korotaev ([email protected])

Kirill

#1 lore raw
Quoted 11 lines · Kirill Korotaev

I remade the previous patch against the latest Linus tree, please apply. This patch fixes strange and obscure pid implementation in current kernels:
- it removes calling of put_task_struct() from detach_pid() under tasklist_lock. This allows to use blocking calls
in security_task_free() hooks (in __put_task_struct()).
- it saves some space = 5*5 ints = 100 bytes in task_struct
- it's smaller and tidy, more straigthforward and doesn't use any knowledge about pids using and assignment.
- it removes pid_links and pid_struct doesn't hold reference counters on task_struct. instead, new pid_structs and linked altogether and only one of them is inserted in hash_list.

Quoted 1 line
Signed-off-by Kirill Korotaev ([email protected])

Could you not rename struct pid and not rename for_each_task_pid()?

Thanks.

-- wli

#2 lore raw
Quoted 12 lines · Kirill Korotaev
Quoted 12 lines

I remade the previous patch against the latest Linus tree, please apply. This patch fixes strange and obscure pid implementation in current kernels:
- it removes calling of put_task_struct() from detach_pid() under tasklist_lock. This allows to use blocking calls
in security_task_free() hooks (in __put_task_struct()).
- it saves some space = 5*5 ints = 100 bytes in task_struct
- it's smaller and tidy, more straigthforward and doesn't use any knowledge about pids using and assignment.
- it removes pid_links and pid_struct doesn't hold reference counters on task_struct. instead, new pid_structs and linked altogether and only one of them is inserted in hash_list.

Signed-off-by Kirill Korotaev ([email protected])
Quoted 1 line · William Lee Irwin III

Could you not rename struct pid and not rename for_each_task_pid()?

And not rename ->hash_chain.

Thanks.

-- wli

#3 lore raw
In reply to William Lee Irwin III
Quoted 12 lines · Kirill Korotaev
Quoted 11 lines

I remade the previous patch against the latest Linus tree, please apply. This patch fixes strange and obscure pid implementation in current kernels:
- it removes calling of put_task_struct() from detach_pid() under tasklist_lock. This allows to use blocking calls
in security_task_free() hooks (in __put_task_struct()).
- it saves some space = 5*5 ints = 100 bytes in task_struct
- it's smaller and tidy, more straigthforward and doesn't use any knowledge about pids using and assignment.
- it removes pid_links and pid_struct doesn't hold reference counters on task_struct. instead, new pid_structs and linked altogether and only one of them is inserted in hash_list.

Quoted 1 line · William Lee Irwin III

Could you not rename struct pid and not rename for_each_task_pid()?

On closer examination for_each_task_pid() appears to need do { ... } while () -like semantics in your scheme, which is nasty as it allows a ne class of mismatching argument errors, but I suppose merits the renaming.

-- wli

#4 lore raw

PATCH 1/7 make do_each_task_pid()/while_each_task_pid() typecheck

Quoted 1 line · William Lee Irwin III
Quoted 1 line

Could you not rename struct pid and not rename for_each_task_pid()?

Quoted 4 lines · William Lee Irwin III

On closer examination for_each_task_pid() appears to need do { ... } while () -like semantics in your scheme, which is nasty as it allows a ne class of mismatching argument errors, but I suppose merits the renaming.

list_empty(&task->pids[type].hash_list) does not successfully typecheck; while this does not correct its semantics, it at least restores typechecking long enough for the code to be examined.

include/linux/pid.h +1 -1
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -49,7 +49,7 @@
 			task = pid_task(task->pids[type].pid_list.next,	\
 						type);			\
 			prefetch(task->pids[type].pid_list.next);	\
-		} while (list_empty(&task->pids[type].hash_list));	\
+		} while (task->pids[type].hash_list.next);	\
 	}
 
 #endif /* _LINUX_PID_H */
#5 lore raw

PATCH 2/7 make do_each_task_pid()/while_each_task_pid() require a semicolon following them

Quoted 3 lines · William Lee Irwin III

list_empty(&task->pids[type].hash_list) does not successfully typecheck; while this does not correct its semantics, it at least restores typechecking long enough for the code to be examined.

do { ... } while () -like constructs require semicolons to follow them. This patch arranges for do_each_task_pid() { ... } while_each_task_pid() to do likewise.

include/linux/pid.h +4 -2
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -41,6 +41,7 @@
 extern void switch_exec_pids(struct task_struct *leader, struct task_struct *thread);
 
 #define do_each_task_pid(who, type, task)				\
+do {									\
 	if ((task = find_task_by_pid_type(type, who))) {		\
 		prefetch(task->pids[type].pid_list.next);		\
 		do {

@@ -49,7 +50,8 @@
 			task = pid_task(task->pids[type].pid_list.next,	\
 						type);			\
 			prefetch(task->pids[type].pid_list.next);	\
-		} while (task->pids[type].hash_list.next);	\
-	}
+		} while (task->pids[type].hash_list.next);		\
+	}								\
+} while (0)
 
 #endif /* _LINUX_PID_H */
#6 lore raw

PATCH 3/7 make do_each_task_pid()/while_each_task_pid() parenthesize their arguments

Quoted 3 lines · William Lee Irwin III

do { ... } while () -like constructs require semicolons to follow them. This patch arranges for do_each_task_pid() { ... } while_each_task_pid() to do likewise.

Macros must parenthesize their arguments when they appear as expressions not surrounded by other bracketing symbols. This patch parenthesizes the task argument to do_each_task_pid() and while_each_task_pid() where needed. This kind of iteration macro doesn't need multiple evaluation fixes.

include/linux/pid.h +4 -4
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -43,14 +43,14 @@
 #define do_each_task_pid(who, type, task)				\
 do {									\
 	if ((task = find_task_by_pid_type(type, who))) {		\
-		prefetch(task->pids[type].pid_list.next);		\
+		prefetch((task)->pids[type].pid_list.next);		\
 		do {
 
 #define while_each_task_pid(who, type, task)				\
-			task = pid_task(task->pids[type].pid_list.next,	\
+			task = pid_task((task)->pids[type].pid_list.next,\
 						type);			\
-			prefetch(task->pids[type].pid_list.next);	\
-		} while (task->pids[type].hash_list.next);		\
+			prefetch((task)->pids[type].pid_list.next);	\
+		} while ((task)->pids[type].hash_list.next);		\
 	}								\
 } while (0)
#7 lore raw

PATCH 4/7 fix loop termination condition in do_each_task_pid()/while_each_task_pid()

Quoted 5 lines · William Lee Irwin III

Macros must parenthesize their arguments when they appear as expressions not surrounded by other bracketing symbols. This patch parenthesizes the task argument to do_each_task_pid() and while_each_task_pid() where needed. This kind of iteration macro doesn't need multiple evaluation fixes.

Now that the basic syntactic issues with the macros have been dealt with, the semantic issue of properly terminating the list iteration remains. The following patch introduces a local variable __list__ in do_each_task_pid() to store the virtual address of the list head of the leader of the collision chain so that the iteration may be properly terminated.

include/linux/pid.h +3 -1
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -42,7 +42,9 @@
 
 #define do_each_task_pid(who, type, task)				\
 do {									\
+	struct list_head *__list__;					\
 	if ((task = find_task_by_pid_type(type, who))) {		\
+		__list__ = &(task)->pids[type].pid_list;		\
 		prefetch((task)->pids[type].pid_list.next);		\
 		do {
 

@@ -50,7 +52,7 @@
 			task = pid_task((task)->pids[type].pid_list.next,\
 						type);			\
 			prefetch((task)->pids[type].pid_list.next);	\
-		} while ((task)->pids[type].hash_list.next);		\
+		} while ((task)->pids[type].pid_list.next != __list__);	\
 	}								\
 } while (0)
#8 lore raw

PATCH 5/7 back out renaming of struct pid

Quoted 6 lines · William Lee Irwin III

Now that the basic syntactic issues with the macros have been dealt with, the semantic issue of properly terminating the list iteration remains. The following patch introduces a local variable __list__ in do_each_task_pid() to store the virtual address of the list head of the leader of the collision chain so that the iteration may be properly terminated.

The renaming of struct pid was spurious; the following patch backs out this renaming.

kernel/sys.c +1 -1
Index: kirill-2.6.9-rc1-mm2/kernel/sys.c
===================================================================
@@ -1165,7 +1165,7 @@
 
 asmlinkage long sys_setsid(void)
 {
-	struct pid_struct *pid;
+	struct pid *pid;
 	int err = -EPERM;
 
 	if (!thread_group_leader(current))
kernel/pid.c +9 -9
Index: kirill-2.6.9-rc1-mm2/kernel/pid.c
===================================================================
@@ -148,10 +148,10 @@
 	return -1;
 }
 
-fastcall struct pid_struct *find_pid(enum pid_type type, int nr)
+struct pid * fastcall find_pid(enum pid_type type, int nr)
 {
 	struct hlist_node *elem;
-	struct pid_struct *pid;
+	struct pid *pid;
 
 	hlist_for_each_entry(pid, elem,
 			&pid_hash[type][pid_hashfn(nr)], hash_list) {

@@ -163,7 +163,7 @@
 
 int fastcall attach_pid(task_t *task, enum pid_type type, int nr)
 {
-	struct pid_struct *pid, *task_pid;
+	struct pid *pid, *task_pid;
 
 	task_pid = &task->pids[type];
 	pid = find_pid(type, nr);

@@ -182,7 +182,7 @@
 
 static inline int __detach_pid(task_t *task, enum pid_type type)
 {
-	struct pid_struct *pid, *pid_next;
+	struct pid *pid, *pid_next;
 	int nr;
 
 	pid = &task->pids[type];

@@ -190,7 +190,7 @@
 		hlist_del(&pid->hash_list);
 		if (!list_empty(&pid->pid_list)) {
 			pid_next = list_entry(pid->pid_list.next,
-						struct pid_struct, pid_list);
+						struct pid, pid_list);
 			/* insert next pid from pid_list to hash */
 			hlist_add_head(&pid_next->hash_list,
 				&pid_hash[type][pid_hashfn(pid_next->nr)]);

@@ -218,11 +218,11 @@
 
 task_t *find_task_by_pid_type(int type, int nr)
 {
-	struct pid_struct *pid = find_pid(type, nr);
+	struct pid *pid = find_pid(type, nr);
 
-	if (!pid)
-		return NULL;
-	return pid_task(&pid->pid_list, type);
+	if (pid)
+		return pid_task(&pid->pid_list, type);
+	return NULL;
 }
 
 EXPORT_SYMBOL(find_task_by_pid_type);
include/linux/sched.h +1 -1
Index: kirill-2.6.9-rc1-mm2/include/linux/sched.h
===================================================================
@@ -503,7 +503,7 @@
 	struct task_struct *group_leader;	/* threadgroup leader */
 
 	/* PID/PID hash table linkage. */
-	struct pid_struct pids[PIDTYPE_MAX];
+	struct pid pids[PIDTYPE_MAX];
 
 	wait_queue_head_t wait_chldexit;	/* for wait4() */
 	struct completion *vfork_done;		/* for vfork() */
include/linux/pid.h +2 -2
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -10,7 +10,7 @@
 	PIDTYPE_MAX
 };
 
-struct pid_struct
+struct pid
 {
 	/* Try to keep hash_list in the same cacheline as nr for find_pid */
 	int nr;

@@ -34,7 +34,7 @@
  * look up a PID in the hash table. Must be called with the tasklist_lock
  * held.
  */
-extern struct pid_struct *FASTCALL(find_pid(enum pid_type, int));
+struct pid *FASTCALL(find_pid(enum pid_type, int));
 
 extern int alloc_pidmap(void);
 extern void FASTCALL(free_pidmap(int));
#9 lore raw

PATCH 6/7 back out renaming of ->pid_chain

Quoted 2 lines · William Lee Irwin III

The renaming of struct pid was spurious; the following patch backs out this renaming.

The renaming of ->pid_chain was spurious; the following patch backs it out.

include/linux/pid.h +2 -2
Index: kirill-2.6.9-rc1-mm2/include/linux/pid.h
===================================================================
@@ -12,9 +12,9 @@
 
 struct pid
 {
-	/* Try to keep hash_list in the same cacheline as nr for find_pid */
+	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
 	int nr;
-	struct hlist_node hash_list;
+	struct hlist_node pid_chain;
 	/* list of pids with the same nr, only one of them is in the hash */
 	struct list_head pid_list;
 };
kernel/pid.c +6 -6
Index: kirill-2.6.9-rc1-mm2/kernel/pid.c
===================================================================
@@ -154,7 +154,7 @@
 	struct pid *pid;
 
 	hlist_for_each_entry(pid, elem,
-			&pid_hash[type][pid_hashfn(nr)], hash_list) {
+			&pid_hash[type][pid_hashfn(nr)], pid_chain) {
 		if (pid->nr == nr)
 			return pid;
 	}

@@ -168,11 +168,11 @@
 	task_pid = &task->pids[type];
 	pid = find_pid(type, nr);
 	if (pid == NULL) {
-		hlist_add_head(&task_pid->hash_list,
+		hlist_add_head(&task_pid->pid_chain,
 				&pid_hash[type][pid_hashfn(nr)]);
 		INIT_LIST_HEAD(&task_pid->pid_list);
 	} else {
-		INIT_HLIST_NODE(&task_pid->hash_list);
+		INIT_HLIST_NODE(&task_pid->pid_chain);
 		list_add_tail(&task_pid->pid_list, &pid->pid_list);
 	}
 	task_pid->nr = nr;

@@ -186,13 +186,13 @@
 	int nr;
 
 	pid = &task->pids[type];
-	if (!hlist_unhashed(&pid->hash_list)) {
-		hlist_del(&pid->hash_list);
+	if (!hlist_unhashed(&pid->pid_chain)) {
+		hlist_del(&pid->pid_chain);
 		if (!list_empty(&pid->pid_list)) {
 			pid_next = list_entry(pid->pid_list.next,
 						struct pid, pid_list);
 			/* insert next pid from pid_list to hash */
-			hlist_add_head(&pid_next->hash_list,
+			hlist_add_head(&pid_next->pid_chain,
 				&pid_hash[type][pid_hashfn(pid_next->nr)]);
 		}
 	}
#10 lore raw

PATCH 7/7 remove casting of __detach_pid() results to void

Quoted 1 line · William Lee Irwin III

The renaming of ->pid_chain was spurious; the following patch backs it out.

Casting an expression to void to ignore the result is only necessary for __must_check function calls and in the interior of nop macros. The following patch backs out the addition of void casts on the result of __detach_pid().

kernel/pid.c +6 -6
Index: kirill-2.6.9-rc1-mm2/kernel/pid.c
===================================================================
@@ -234,13 +234,13 @@
  */
 void switch_exec_pids(task_t *leader, task_t *thread)
 {
-	(void)__detach_pid(leader, PIDTYPE_PID);
-	(void)__detach_pid(leader, PIDTYPE_TGID);
-	(void)__detach_pid(leader, PIDTYPE_PGID);
-	(void)__detach_pid(leader, PIDTYPE_SID);
+	__detach_pid(leader, PIDTYPE_PID);
+	__detach_pid(leader, PIDTYPE_TGID);
+	__detach_pid(leader, PIDTYPE_PGID);
+	__detach_pid(leader, PIDTYPE_SID);
 
-	(void)__detach_pid(thread, PIDTYPE_PID);
-	(void)__detach_pid(thread, PIDTYPE_TGID);
+	__detach_pid(thread, PIDTYPE_PID);
+	__detach_pid(thread, PIDTYPE_TGID);
 
 	leader->pid = leader->tgid = thread->pid;
 	thread->pid = thread->tgid;
#11 lore raw

Re: [1/7] make do_each_task_pid()/while_each_task_pid() typecheck

In reply to William Lee Irwin III

Heh. These improvements look fine, but I definitely don't want to first apply the broken one and then improve upon it - it would be much nicer to get this kind of nice "progression" patch that starts off from a clean thing and just improves on it (maybe that ends up meaning just one patch to replace Kirill's, I don't know..)

Linus

#12 lore raw

PATCH 1/1 rework of Kirill Korotaev's pidhashing patch

Quoted 5 lines · Linus Torvalds

Heh. These improvements look fine, but I definitely don't want to first apply the broken one and then improve upon it - it would be much nicer to get this kind of nice "progression" patch that starts off from a clean thing and just improves on it (maybe that ends up meaning just one patch to replace Kirill's, I don't know..)

Address a correctness issue with loop termination in do_each_task_pid() and while_each_task_pid() as well as various stylistic and macro hygiene issues with the original. Otherwise unaltered. The description from Kirill Korotaev's original post is as follows:

"This patch fixes strange and obscure pid implementation in current kernels:
- it removes calling of put_task_struct() from detach_pid() under tasklist_lock. This allows to use blocking calls
in security_task_free() hooks (in __put_task_struct()).
- it saves some space = 5*5 ints = 100 bytes in task_struct
- it's smaller and tidy, more straigthforward and doesn't use any knowledge about pids using and assignment.
- it removes pid_links and pid_struct doesn't hold reference counters on task_struct. instead, new pid_structs and linked altogether and only one of them is inserted in hash_list."

I've only compiletested my changes, though I have strong reasons to believe they're correct. If Kirill signs off on this, I will too.

drivers/char/tty_io.c +12 -18
Index: kirill-2.6.9-rc1-mm2/drivers/char/tty_io.c
===================================================================
@@ -424,7 +424,6 @@
 	struct file * cons_filp = NULL;
 	struct file *filp, *f = NULL;
 	struct task_struct *p;
-	struct pid *pid;
 	int    closecount = 0, n;
 
 	if (!tty)

@@ -487,8 +486,7 @@
 	
 	read_lock(&tasklist_lock);
 	if (tty->session > 0) {
-		struct list_head *l;
-		for_each_task_pid(tty->session, PIDTYPE_SID, p, l, pid) {
+		do_each_task_pid(tty->session, PIDTYPE_SID, p) {
 			if (p->signal->tty == tty)
 				p->signal->tty = NULL;
 			if (!p->signal->leader)

@@ -497,7 +495,7 @@
 			send_group_sig_info(SIGCONT, SEND_SIG_PRIV, p);
 			if (tty->pgrp > 0)
 				p->signal->tty_old_pgrp = tty->pgrp;
-		}
+		} while_each_task_pid(tty->session, PIDTYPE_SID, p);
 	}
 	read_unlock(&tasklist_lock);
 

@@ -569,8 +567,6 @@
 {
 	struct tty_struct *tty;
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 	int tty_pgrp = -1;
 
 	lock_kernel();

@@ -599,8 +595,9 @@
 	tty->pgrp = -1;
 
 	read_lock(&tasklist_lock);
-	for_each_task_pid(current->signal->session, PIDTYPE_SID, p, l, pid)
+	do_each_task_pid(current->signal->session, PIDTYPE_SID, p) {
 		p->signal->tty = NULL;
+	} while_each_task_pid(current->signal->session, PIDTYPE_SID, p);
 	read_unlock(&tasklist_lock);
 	unlock_kernel();
 }

@@ -1252,15 +1249,15 @@
 	 */
 	if (tty_closing || o_tty_closing) {
 		struct task_struct *p;
-		struct list_head *l;
-		struct pid *pid;
 
 		read_lock(&tasklist_lock);
-		for_each_task_pid(tty->session, PIDTYPE_SID, p, l, pid)
+		do_each_task_pid(tty->session, PIDTYPE_SID, p) {
 			p->signal->tty = NULL;
+		} while_each_task_pid(tty->session, PIDTYPE_SID, p);
 		if (o_tty)
-			for_each_task_pid(o_tty->session, PIDTYPE_SID, p,l, pid)
+			do_each_task_pid(o_tty->session, PIDTYPE_SID, p) {
 				p->signal->tty = NULL;
+			} while_each_task_pid(o_tty->session, PIDTYPE_SID, p);
 		read_unlock(&tasklist_lock);
 	}
 

@@ -1630,8 +1627,6 @@
 
 static int tiocsctty(struct tty_struct *tty, int arg)
 {
-	struct list_head *l;
-	struct pid *pid;
 	task_t *p;
 
 	if (current->signal->leader &&

@@ -1654,8 +1649,9 @@
 			 */
 
 			read_lock(&tasklist_lock);
-			for_each_task_pid(tty->session, PIDTYPE_SID, p, l, pid)
+			do_each_task_pid(tty->session, PIDTYPE_SID, p) {
 				p->signal->tty = NULL;
+			} while_each_task_pid(tty->session, PIDTYPE_SID, p);
 			read_unlock(&tasklist_lock);
 		} else
 			return -EPERM;

@@ -1962,8 +1958,6 @@
 #else
 	struct tty_struct *tty = arg;
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 	int session;
 	int		i;
 	struct file	*filp;

@@ -1976,7 +1970,7 @@
 	if (tty->driver->flush_buffer)
 		tty->driver->flush_buffer(tty);
 	read_lock(&tasklist_lock);
-	for_each_task_pid(session, PIDTYPE_SID, p, l, pid) {
+	do_each_task_pid(session, PIDTYPE_SID, p) {
 		if (p->signal->tty == tty || session > 0) {
 			printk(KERN_NOTICE "SAK: killed process %d"
 			    " (%s): p->signal->session==tty->session\n",

@@ -2003,7 +1997,7 @@
 			spin_unlock(&p->files->file_lock);
 		}
 		task_unlock(p);
-	}
+	} while_each_task_pid(session, PIDTYPE_SID, p);
 	read_unlock(&tasklist_lock);
 #endif
 }
fs/fcntl.c +4 -8
Index: kirill-2.6.9-rc1-mm2/fs/fcntl.c
===================================================================
@@ -497,11 +497,9 @@
 			send_sigio_to_task(p, fown, fd, band);
 		}
 	} else {
-		struct list_head *l;
-		struct pid *pidptr;
-		for_each_task_pid(-pid, PIDTYPE_PGID, p, l, pidptr) {
+		do_each_task_pid(-pid, PIDTYPE_PGID, p) {
 			send_sigio_to_task(p, fown, fd, band);
-		}
+		} while_each_task_pid(-pid, PIDTYPE_PGID, p);
 	}
 	read_unlock(&tasklist_lock);
  out_unlock_fown:

@@ -534,11 +532,9 @@
 			send_sigurg_to_task(p, fown);
 		}
 	} else {
-		struct list_head *l;
-		struct pid *pidptr;
-		for_each_task_pid(-pid, PIDTYPE_PGID, p, l, pidptr) {
+		do_each_task_pid(-pid, PIDTYPE_PGID, p) {
 			send_sigurg_to_task(p, fown);
-		}
+		} while_each_task_pid(-pid, PIDTYPE_PGID, p);
 	}
 	read_unlock(&tasklist_lock);
  out_unlock_fown:
fs/proc/base.c +2 -3
Index: kirill-2.6.9-rc1-mm2/fs/proc/base.c
===================================================================
@@ -792,10 +792,9 @@
 	.follow_link	= proc_pid_follow_link
 };
 
-static int pid_alive(struct task_struct *p)
+static inline int pid_alive(struct task_struct *p)
 {
-	BUG_ON(p->pids[PIDTYPE_PID].pidptr != &p->pids[PIDTYPE_PID].pid);
-	return atomic_read(&p->pids[PIDTYPE_PID].pid.count);
+	return p->pids[PIDTYPE_PID].nr != 0;
 }
 
 #define NUMBUF 10
include/linux/pid.h +22 -28

76 lines not on this page · raw message

include/linux/sched.h +4 -5
Index: kirill-2.6.9-rc1-mm2/include/linux/sched.h
===================================================================
@@ -503,7 +503,7 @@
 	struct task_struct *group_leader;	/* threadgroup leader */
 
 	/* PID/PID hash table linkage. */
-	struct pid_link pids[PIDTYPE_MAX];
+	struct pid pids[PIDTYPE_MAX];
 
 	wait_queue_head_t wait_chldexit;	/* for wait4() */
 	struct completion *vfork_done;		/* for vfork() */

@@ -695,7 +695,8 @@
 
 extern struct   mm_struct init_mm;
 
-extern struct task_struct *find_task_by_pid(int pid);
+#define find_task_by_pid(nr)	find_task_by_pid_type(PIDTYPE_PID, nr)
+extern struct task_struct *find_task_by_pid_type(int type, int pid);
 extern void set_special_pids(pid_t session, pid_t pgrp);
 extern void __set_special_pids(pid_t session, pid_t pgrp);
 

@@ -897,9 +898,7 @@
 
 static inline int thread_group_empty(task_t *p)
 {
-	struct pid *pid = p->pids[PIDTYPE_TGID].pidptr;
-
-	return pid->task_list.next->next == &pid->task_list;
+	return list_empty(&p->pids[PIDTYPE_TGID].pid_list);
 }
 
 #define delay_group_leader(p) \
kernel/capability.c +2 -4
Index: kirill-2.6.9-rc1-mm2/kernel/capability.c
===================================================================
@@ -89,14 +89,12 @@
 			      kernel_cap_t *permitted)
 {
 	task_t *g, *target;
-	struct list_head *l;
-	struct pid *pid;
 
-	for_each_task_pid(pgrp, PIDTYPE_PGID, g, l, pid) {
+	do_each_task_pid(pgrp, PIDTYPE_PGID, g) {
 		target = g;
 		while_each_thread(g, target)
 			security_capset_set(target, effective, inheritable, permitted);
-	}
+	} while_each_task_pid(pgrp, PIDTYPE_PGID, g);
 }
 
 /*
kernel/exit.c +7 -19
Index: kirill-2.6.9-rc1-mm2/kernel/exit.c
===================================================================
@@ -128,16 +128,15 @@
 int session_of_pgrp(int pgrp)
 {
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 	int sid = -1;
 
 	read_lock(&tasklist_lock);
-	for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid)
+	do_each_task_pid(pgrp, PIDTYPE_PGID, p) {
 		if (p->signal->session > 0) {
 			sid = p->signal->session;
 			goto out;
 		}
+	} while_each_task_pid(pgrp, PIDTYPE_PGID, p);
 	p = find_task_by_pid(pgrp);
 	if (p)
 		sid = p->signal->session;

@@ -158,11 +157,9 @@
 static int will_become_orphaned_pgrp(int pgrp, task_t *ignored_task)
 {
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 	int ret = 1;
 
-	for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
+	do_each_task_pid(pgrp, PIDTYPE_PGID, p) {
 		if (p == ignored_task
 				|| p->state >= TASK_ZOMBIE 
 				|| p->real_parent->pid == 1)

@@ -172,7 +169,7 @@
 			ret = 0;
 			break;
 		}
-	}
+	} while_each_task_pid(pgrp, PIDTYPE_PGID, p);
 	return ret;	/* (sighing) "Often!" */
 }
 

@@ -191,15 +188,13 @@
 {
 	int retval = 0;
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 
-	for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
+	do_each_task_pid(pgrp, PIDTYPE_PGID, p) {
 		if (p->state != TASK_STOPPED)
 			continue;
 		retval = 1;
 		break;
-	}
+	} while_each_task_pid(pgrp, PIDTYPE_PGID, p);
 	return retval;
 }
 

@@ -852,9 +847,6 @@
 
 task_t fastcall *next_thread(const task_t *p)
 {
-	const struct pid_link *link = p->pids + PIDTYPE_TGID;
-	const struct list_head *tmp, *head = &link->pidptr->task_list;
-
 #ifdef CONFIG_SMP
 	if (!p->sighand)
 		BUG();

@@ -862,11 +854,7 @@
 				!rwlock_is_locked(&tasklist_lock))
 		BUG();
 #endif
-	tmp = link->pid_chain.next;
-	if (tmp == head)
-		tmp = head->next;
-
-	return pid_task(tmp, PIDTYPE_TGID);
+	return pid_task(p->pids[PIDTYPE_TGID].pid_list.next, PIDTYPE_TGID);
 }
 
 EXPORT_SYMBOL(next_thread);
kernel/fork.c +2 -3
Index: kirill-2.6.9-rc1-mm2/kernel/fork.c
===================================================================
@@ -1011,14 +1011,13 @@
 	cpuset_fork(p);
 
 	attach_pid(p, PIDTYPE_PID, p->pid);
+	attach_pid(p, PIDTYPE_TGID, p->tgid);
 	if (thread_group_leader(p)) {
-		attach_pid(p, PIDTYPE_TGID, p->tgid);
 		attach_pid(p, PIDTYPE_PGID, process_group(p));
 		attach_pid(p, PIDTYPE_SID, p->signal->session);
 		if (p->pid)
 			__get_cpu_var(process_counts)++;
-	} else
-		link_pid(p, p->pids + PIDTYPE_TGID, &p->group_leader->pids[PIDTYPE_TGID].pid);
+	}
 
 	nr_threads++;
 	write_unlock_irq(&tasklist_lock);
kernel/pid.c +38 -47

137 lines not on this page · raw message

kernel/signal.c +4 -8
Index: kirill-2.6.9-rc1-mm2/kernel/signal.c
===================================================================
@@ -1115,8 +1115,6 @@
 int __kill_pg_info(int sig, struct siginfo *info, pid_t pgrp)
 {
 	struct task_struct *p;
-	struct list_head *l;
-	struct pid *pid;
 	int retval, success;
 
 	if (pgrp <= 0)

@@ -1124,11 +1122,11 @@
 
 	success = 0;
 	retval = -ESRCH;
-	for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
+	do_each_task_pid(pgrp, PIDTYPE_PGID, p) {
 		int err = group_send_sig_info(sig, info, p);
 		success |= !err;
 		retval = err;
-	}
+	} while_each_task_pid(pgrp, PIDTYPE_PGID, p);
 	return success ? 0 : retval;
 }
 

@@ -1155,8 +1153,6 @@
 kill_sl_info(int sig, struct siginfo *info, pid_t sid)
 {
 	int err, retval = -EINVAL;
-	struct pid *pid;
-	struct list_head *l;
 	struct task_struct *p;
 
 	if (sid <= 0)

@@ -1164,13 +1160,13 @@
 
 	retval = -ESRCH;
 	read_lock(&tasklist_lock);
-	for_each_task_pid(sid, PIDTYPE_SID, p, l, pid) {
+	do_each_task_pid(sid, PIDTYPE_SID, p) {
 		if (!p->signal->leader)
 			continue;
 		err = group_send_sig_info(sig, info, p);
 		if (retval)
 			retval = err;
-	}
+	} while_each_task_pid(sid, PIDTYPE_SID, p);
 	read_unlock(&tasklist_lock);
 out:
 	return retval;
kernel/sys.c +6 -10
Index: kirill-2.6.9-rc1-mm2/kernel/sys.c
===================================================================
@@ -320,8 +320,6 @@
 {
 	struct task_struct *g, *p;
 	struct user_struct *user;
-	struct pid *pid;
-	struct list_head *l;
 	int error = -EINVAL;
 
 	if (which > 2 || which < 0)

@@ -346,8 +344,9 @@
 		case PRIO_PGRP:
 			if (!who)
 				who = process_group(current);
-			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid)
+			do_each_task_pid(who, PIDTYPE_PGID, p) {
 				error = set_one_prio(p, niceval, error);
+			} while_each_task_pid(who, PIDTYPE_PGID, p);
 			break;
 		case PRIO_USER:
 			if (!who)

@@ -381,8 +380,6 @@
 asmlinkage long sys_getpriority(int which, int who)
 {
 	struct task_struct *g, *p;
-	struct list_head *l;
-	struct pid *pid;
 	struct user_struct *user;
 	long niceval, retval = -ESRCH;
 

@@ -404,11 +401,11 @@
 		case PRIO_PGRP:
 			if (!who)
 				who = process_group(current);
-			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid) {
+			do_each_task_pid(who, PIDTYPE_PGID, p) {
 				niceval = 20 - task_nice(p);
 				if (niceval > retval)
 					retval = niceval;
-			}
+			} while_each_task_pid(who, PIDTYPE_PGID, p);
 			break;
 		case PRIO_USER:
 			if (!who)

@@ -1086,12 +1083,11 @@
 
 	if (pgid != pid) {
 		struct task_struct *p;
-		struct pid *pid;
-		struct list_head *l;
 
-		for_each_task_pid(pgid, PIDTYPE_PGID, p, l, pid)
+		do_each_task_pid(pgid, PIDTYPE_PGID, p) {
 			if (p->signal->session == current->signal->session)
 				goto ok_pgid;
+		} while_each_task_pid(pgid, PIDTYPE_PGID, p);
 		goto out;
 	}