Re: [PATCH] pids: simplify do_each_task_pid/while_each_task_pid

[Christoph Hellwig <hch@infradead.org>]

On Thu, Apr 13, 2006 at 09:54:31PM +0400, Oleg Nesterov wrote:
> > 
> > #define for_each_task_pid(task, pid, type, pos) \
> > 	hlist_for_each_entry_rcu((task), (pos),  \
> > 		(&(pid))->tasks[type], pids[type].node) {
> > 
> > and move the find_pid to the caller?  That would make the code a whole lot
> > more readable.
> 
> Then the caller should check find_pid() doesn't return NULL. But yes,
> we can hide this check inside for_each_task_pid().
> 
> But what about current users of do_each_task_pid ? We can't just remove
> these macros.

They'd have to switch over to the new variant.  There's just 18 callers
ayway, currently, and with a patch like the one below that number firther
decreases :)


Index: linux-2.6/drivers/char/tty_io.c
===================================================================
--- linux-2.6.orig/drivers/char/tty_io.c	2006-04-13 16:22:12.000000000 +0200
+++ linux-2.6/drivers/char/tty_io.c	2006-04-13 16:39:57.000000000 +0200
@@ -1174,6 +1174,17 @@
 
 EXPORT_SYMBOL(tty_hung_up_p);
 
+static void clear_session_ttys(pid_t session)
+{
+	struct task_struct *p;
+
+	read_lock(&tasklist_lock);
+	do_each_task_pid(session, PIDTYPE_SID, p) {
+		p->signal->tty = NULL;
+	} while_each_task_pid(session, PIDTYPE_SID, p);
+	read_unlock(&tasklist_lock);
+}
+
 /*
  * This function is typically called only by the session leader, when
  * it wants to disassociate itself from its controlling tty.
@@ -1190,7 +1201,6 @@
 void disassociate_ctty(int on_exit)
 {
 	struct tty_struct *tty;
-	struct task_struct *p;
 	int tty_pgrp = -1;
 
 	lock_kernel();
@@ -1224,11 +1234,7 @@
 	tty->pgrp = -1;
 
 	/* Now clear signal->tty under the lock */
-	read_lock(&tasklist_lock);
-	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);
+	clear_session_ttys(current->signal->session);
 	mutex_unlock(&tty_mutex);
 	unlock_kernel();
 }
@@ -1927,17 +1933,9 @@
 	 * tty.
 	 */
 	if (tty_closing || o_tty_closing) {
-		struct task_struct *p;
-
-		read_lock(&tasklist_lock);
-		do_each_task_pid(tty->session, PIDTYPE_SID, p) {
-			p->signal->tty = NULL;
-		} while_each_task_pid(tty->session, PIDTYPE_SID, p);
+		clear_session_ttys(tty->session);
 		if (o_tty)
-			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);
+			clear_session_ttys(o_tty->session);
 	}
 
 	mutex_unlock(&tty_mutex);
@@ -2348,8 +2346,6 @@
 
 static int tiocsctty(struct tty_struct *tty, int arg)
 {
-	task_t *p;
-
 	if (current->signal->leader &&
 	    (current->signal->session == tty->session))
 		return 0;
@@ -2364,18 +2360,12 @@
 		 * This tty is already the controlling
 		 * tty for another session group!
 		 */
-		if ((arg == 1) && capable(CAP_SYS_ADMIN)) {
-			/*
-			 * Steal it away
-			 */
-
-			read_lock(&tasklist_lock);
-			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
+		if (arg != 1 || !capable(CAP_SYS_ADMIN))
 			return -EPERM;
+		/*
+		 * Steal it away
+		 */
+		clear_session_ttys(tty->session);
 	}
 	task_lock(current);
 	current->signal->tty = tty;