Re: [PATCH] pids: simplify do_each_task_pid/while_each_task_pid

Christoph Hellwig
2006-04-13T15:07:30Z

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;