use group_leader->pgrp (was Re: setpgid and threads)

PATCH In review Jeremy Fitzhardinge 4 participants 9 messages Sep 9, 2003 2 messages not in the archive

Subsystems (10)

VFS MIPS MM core Scheduler +6 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jeremy Fitzhardinge

author

2003-09-09 02:02 UTC #2 lore raw

PATCH 1/1 use group_leader->pgrp (was Re: setpgid and threads)

Quoted 1 line · Andrew Morton

Using current->thread_group->foo sounds like a reasonable solution

Here's a patch to use group_leader->pgrp rather than just ->pgrp so that all threads in a group appear to have the same process group ID. This is against 2.6.0-test4-mm6, and so it backs out the previous patch to kernel/sys.c.

I'm running this now, and it seems to work, including the original test with which I noticed the original problem. I can't claim to have heavily tested it though. I think I've compiled all the affected code, except the non-x86 arch-specific stuff. The only mildly subtle stuff seems to be in around the handling of execve() in kernel/pid.c.

 arch/h8300/kernel/signal.c     |    2 +-
 arch/m68k/kernel/signal.c      |    2 +-
 arch/m68knommu/kernel/signal.c |    2 +-
 arch/mips/kernel/irixelf.c     |    2 +-
 arch/mips/kernel/irixsig.c     |    2 +-
 arch/mips/kernel/sysirix.c     |    4 ++--
 arch/sparc64/solaris/misc.c    |    4 ++--
 drivers/char/n_tty.c           |    6 +++---
 drivers/char/rocket.c          |    2 +-
 drivers/char/tty_io.c          |    2 +-
 fs/autofs/autofs_i.h           |    2 +-
 fs/autofs/inode.c              |    2 +-
 fs/autofs/root.c               |    4 ++--
 fs/autofs4/autofs_i.h          |    2 +-
 fs/autofs4/inode.c             |    4 ++--
 fs/autofs4/root.c              |    4 ++--
 fs/binfmt_elf.c                |    4 ++--
 fs/coda/upcall.c               |    2 +-
 fs/devfs/base.c                |    6 +++---
 fs/proc/array.c                |    2 +-
 kernel/exit.c                  |   20 ++++++++++----------
 kernel/pid.c                   |    2 +-
 kernel/signal.c                |    4 ++--
 kernel/sys.c                   |   21 +++++----------------
 24 files changed, 48 insertions(+), 59 deletions(-)

arch/h8300/kernel/signal.c renamed from arch/h8300/kernel/signal.c~pgrp +1 -1

diff -puN arch/h8300/kernel/signal.c~pgrp arch/h8300/kernel/signal.c
@@ -593,7 +593,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(current->group_leader->pgrp))
 					continue;
 				/* FALLTHRU */

arch/m68k/kernel/signal.c renamed from arch/m68k/kernel/signal.c~pgrp +1 -1

diff -puN arch/m68k/kernel/signal.c~pgrp arch/m68k/kernel/signal.c
@@ -1082,7 +1082,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(current->group_leader->pgrp))
 					continue;
 				/* FALLTHRU */

arch/m68knommu/kernel/signal.c renamed from arch/m68knommu/kernel/signal.c~pgrp +1 -1

diff -puN arch/m68knommu/kernel/signal.c~pgrp arch/m68knommu/kernel/signal.c
@@ -841,7 +841,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(current->group_leader->pgrp))
 					continue;
 				/* FALLTHRU */

arch/mips/kernel/irixelf.c renamed from arch/mips/kernel/irixelf.c~pgrp +1 -1

diff -puN arch/mips/kernel/irixelf.c~pgrp arch/mips/kernel/irixelf.c
@@ -1129,7 +1129,7 @@ static int irix_core_dump(long signr, st
 	prstatus.pr_sighold = current->blocked.sig[0];
 	psinfo.pr_pid = prstatus.pr_pid = current->pid;
 	psinfo.pr_ppid = prstatus.pr_ppid = current->parent->pid;
-	psinfo.pr_pgrp = prstatus.pr_pgrp = current->pgrp;
+	psinfo.pr_pgrp = prstatus.pr_pgrp = current->group_leader->pgrp;
 	psinfo.pr_sid = prstatus.pr_sid = current->session;
 	prstatus.pr_utime.tv_sec = CT_TO_SECS(current->utime);
 	prstatus.pr_utime.tv_usec = CT_TO_USECS(current->utime);

arch/mips/kernel/irixsig.c renamed from arch/mips/kernel/irixsig.c~pgrp +1 -1

diff -puN arch/mips/kernel/irixsig.c~pgrp arch/mips/kernel/irixsig.c
@@ -582,7 +582,7 @@ repeat:
 		p = list_entry(_p,struct task_struct,sibling);
 		if ((type == P_PID) && p->pid != pid)
 			continue;
-		if ((type == P_PGID) && p->pgrp != pid)
+		if ((type == P_PGID) && p->group_leader->pgrp != pid)
 			continue;
 		if ((p->exit_signal != SIGCHLD))
 			continue;

arch/mips/kernel/sysirix.c renamed from arch/mips/kernel/sysirix.c~pgrp +2 -2

diff -puN arch/mips/kernel/sysirix.c~pgrp arch/mips/kernel/sysirix.c
@@ -803,11 +803,11 @@ asmlinkage int irix_setpgrp(int flags)
 	printk("[%s:%d] setpgrp(%d) ", current->comm, current->pid, flags);
 #endif
 	if(!flags)
-		error = current->pgrp;
+		error = current->group_leader->pgrp;
 	else
 		error = sys_setsid();
 #ifdef DEBUG_PROCGRPS
-	printk("returning %d\n", current->pgrp);
+	printk("returning %d\n", current->group_leader->pgrp);
 #endif
 
 	return error;

arch/sparc64/solaris/misc.c renamed from arch/sparc64/solaris/misc.c~pgrp +2 -2

diff -puN arch/sparc64/solaris/misc.c~pgrp arch/sparc64/solaris/misc.c
@@ -392,7 +392,7 @@ asmlinkage int solaris_procids(int cmd, 
 	
 	switch (cmd) {
 	case 0: /* getpgrp */
-		return current->pgrp;
+		return current->group_leader->pgrp;
 	case 1: /* setpgrp */
 		{
 			int (*sys_setpgid)(pid_t,pid_t) =

@@ -403,7 +403,7 @@ asmlinkage int solaris_procids(int cmd, 
 			ret = sys_setpgid(0, 0);
 			if (ret) return ret;
 			current->tty = NULL;
-			return current->pgrp;
+			return current->group_leader->pgrp;
 		}
 	case 2: /* getsid */
 		{

drivers/char/n_tty.c renamed from drivers/char/n_tty.c~pgrp +3 -3

diff -puN drivers/char/n_tty.c~pgrp drivers/char/n_tty.c
@@ -977,11 +977,11 @@ do_it_again:
 	if (file->f_op->write != redirected_tty_write && current->tty == tty) {
 		if (tty->pgrp <= 0)
 			printk("read_chan: tty->pgrp <= 0!\n");
-		else if (current->pgrp != tty->pgrp) {
+		else if (current->group_leader->pgrp != tty->pgrp) {
 			if (is_ignored(SIGTTIN) ||
-			    is_orphaned_pgrp(current->pgrp))
+			    is_orphaned_pgrp(current->group_leader->pgrp))
 				return -EIO;
-			kill_pg(current->pgrp, SIGTTIN, 1);
+			kill_pg(current->group_leader->pgrp, SIGTTIN, 1);
 			return -ERESTARTSYS;
 		}
 	}

drivers/char/rocket.c renamed from drivers/char/rocket.c~pgrp +1 -1

diff -puN drivers/char/rocket.c~pgrp drivers/char/rocket.c
@@ -956,7 +956,7 @@ static int rp_open(struct tty_struct *tt
 	 * Info->count is now 1; so it's safe to sleep now.
 	 */
 	info->session = current->session;
-	info->pgrp = current->pgrp;
+	info->pgrp = current->group_leader->pgrp;
 
 	if ((info->flags & ROCKET_INITIALIZED) == 0) {
 		cp = &info->channel;

drivers/char/tty_io.c renamed from drivers/char/tty_io.c~pgrp +1 -1

diff -puN drivers/char/tty_io.c~pgrp drivers/char/tty_io.c
@@ -325,7 +325,7 @@ int tty_check_change(struct tty_struct *
 		printk(KERN_WARNING "tty_check_change: tty->pgrp <= 0!\n");
 		return 0;
 	}
-	if (current->pgrp == tty->pgrp)
+	if (current->group_leader->pgrp == tty->pgrp)
 		return 0;
 	if (is_ignored(SIGTTOU))
 		return 0;

fs/autofs/autofs_i.h renamed from fs/autofs/autofs_i.h~pgrp +1 -1

diff -puN fs/autofs/autofs_i.h~pgrp fs/autofs/autofs_i.h
@@ -123,7 +123,7 @@ static inline struct autofs_sb_info *aut
    filesystem without "magic".) */
 
 static inline int autofs_oz_mode(struct autofs_sb_info *sbi) {
-	return sbi->catatonic || current->pgrp == sbi->oz_pgrp;
+	return sbi->catatonic || current->group_leader->pgrp == sbi->oz_pgrp;
 }
 
 /* Hash operations */

fs/autofs/inode.c renamed from fs/autofs/inode.c~pgrp +1 -1

diff -puN fs/autofs/inode.c~pgrp fs/autofs/inode.c
@@ -51,7 +51,7 @@ static int parse_options(char *options, 
 	
 	*uid = current->uid;
 	*gid = current->gid;
-	*pgrp = current->pgrp;
+	*pgrp = current->group_leader->pgrp;
 
 	*minproto = *maxproto = AUTOFS_PROTO_VERSION;

fs/autofs/root.c renamed from fs/autofs/root.c~pgrp +2 -2

diff -puN fs/autofs/root.c~pgrp fs/autofs/root.c
@@ -213,7 +213,7 @@ static struct dentry *autofs_root_lookup
 
 	oz_mode = autofs_oz_mode(sbi);
 	DPRINTK(("autofs_lookup: pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
-		 current->pid, current->pgrp, sbi->catatonic, oz_mode));
+		 current->pid, current->group_leader->pgrp, sbi->catatonic, oz_mode));
 
 	/*
 	 * Mark the dentry incomplete, but add it. This is needed so

@@ -527,7 +527,7 @@ static int autofs_root_ioctl(struct inod
 {
 	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
 
-	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",cmd,arg,sbi,current->pgrp));
+	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",cmd,arg,sbi,current->group_leader->pgrp));
 
 	if ( _IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) ||
 	     _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT )

fs/autofs4/autofs_i.h renamed from fs/autofs4/autofs_i.h~pgrp +1 -1

diff -puN fs/autofs4/autofs_i.h~pgrp fs/autofs4/autofs_i.h
@@ -113,7 +113,7 @@ static inline struct autofs_info *autofs
    filesystem without "magic".) */
 
 static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) {
-	return sbi->catatonic || current->pgrp == sbi->oz_pgrp;
+	return sbi->catatonic || current->group_leader->pgrp == sbi->oz_pgrp;
 }
 
 /* Does a dentry have some pending activity? */

fs/autofs4/inode.c renamed from fs/autofs4/inode.c~pgrp +2 -2

diff -puN fs/autofs4/inode.c~pgrp fs/autofs4/inode.c
@@ -101,7 +101,7 @@ static int parse_options(char *options, 
 	
 	*uid = current->uid;
 	*gid = current->gid;
-	*pgrp = current->pgrp;
+	*pgrp = current->group_leader->pgrp;
 
 	*minproto = AUTOFS_MIN_PROTO_VERSION;
 	*maxproto = AUTOFS_MAX_PROTO_VERSION;

@@ -192,7 +192,7 @@ int autofs4_fill_super(struct super_bloc
 	sbi->magic = AUTOFS_SBI_MAGIC;
 	sbi->catatonic = 0;
 	sbi->exp_timeout = 0;
-	sbi->oz_pgrp = current->pgrp;
+	sbi->oz_pgrp = current->group_leader->pgrp;
 	sbi->sb = s;
 	sbi->version = 0;
 	sbi->queues = NULL;

fs/autofs4/root.c renamed from fs/autofs4/root.c~pgrp +2 -2

diff -puN fs/autofs4/root.c~pgrp fs/autofs4/root.c
@@ -255,7 +255,7 @@ static struct dentry *autofs4_root_looku
 	lock_kernel();
 	oz_mode = autofs4_oz_mode(sbi);
 	DPRINTK(("autofs_lookup: pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
-		 current->pid, current->pgrp, sbi->catatonic, oz_mode));
+		 current->pid, current->group_leader->pgrp, sbi->catatonic, oz_mode));
 
 	/*
 	 * Mark the dentry incomplete, but add it. This is needed so

@@ -518,7 +518,7 @@ static int autofs4_root_ioctl(struct ino
 	struct autofs_sb_info *sbi = autofs4_sbi(inode->i_sb);
 
 	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",
-		 cmd,arg,sbi,current->pgrp));
+		 cmd,arg,sbi,current->group_leader->pgrp));
 
 	if ( _IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) ||
 	     _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT )

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~pgrp +2 -2

diff -puN fs/binfmt_elf.c~pgrp fs/binfmt_elf.c
@@ -1076,7 +1076,7 @@ static void fill_prstatus(struct elf_prs
 	prstatus->pr_sighold = p->blocked.sig[0];
 	prstatus->pr_pid = p->pid;
 	prstatus->pr_ppid = p->parent->pid;
-	prstatus->pr_pgrp = p->pgrp;
+	prstatus->pr_pgrp = p->group_leader->pgrp;
 	prstatus->pr_sid = p->session;
 	jiffies_to_timeval(p->utime, &prstatus->pr_utime);
 	jiffies_to_timeval(p->stime, &prstatus->pr_stime);

@@ -1104,7 +1104,7 @@ static void fill_psinfo(struct elf_prpsi
 
 	psinfo->pr_pid = p->pid;
 	psinfo->pr_ppid = p->parent->pid;
-	psinfo->pr_pgrp = p->pgrp;
+	psinfo->pr_pgrp = p->group_leader->pgrp;
 	psinfo->pr_sid = p->session;
 
 	i = p->state ? ffz(~p->state) + 1 : 0;

fs/coda/upcall.c renamed from fs/coda/upcall.c~pgrp +1 -1

diff -puN fs/coda/upcall.c~pgrp fs/coda/upcall.c
@@ -54,7 +54,7 @@ static void *alloc_upcall(int opcode, in
 
         inp->ih.opcode = opcode;
 	inp->ih.pid = current->pid;
-	inp->ih.pgid = current->pgrp;
+	inp->ih.pgid = current->group_leader->pgrp;
 	coda_load_creds(&(inp->ih.cred));
 
 	return (void*)inp;

fs/devfs/base.c renamed from fs/devfs/base.c~pgrp +3 -3

diff -puN fs/devfs/base.c~pgrp fs/devfs/base.c
@@ -1334,7 +1334,7 @@ static int is_devfsd_or_child (struct fs
     struct task_struct *p = current;
 
     if (p == fs_info->devfsd_task) return (TRUE);
-    if (p->pgrp == fs_info->devfsd_pgrp) return (TRUE);
+    if (p->group_leader->pgrp == fs_info->devfsd_pgrp) return (TRUE);
     read_lock(&tasklist_lock);
     for ( ; p != &init_task; p = p->real_parent)
     {

@@ -2745,8 +2745,8 @@ static int devfsd_ioctl (struct inode *i
 	    }
 	    fs_info->devfsd_task = current;
 	    spin_unlock (&lock);
-	    fs_info->devfsd_pgrp = (current->pgrp == current->pid) ?
-		current->pgrp : 0;
+	    fs_info->devfsd_pgrp = (current->group_leader->pgrp == current->pid) ?
+		current->group_leader->pgrp : 0;
 	    fs_info->devfsd_file = file;
 	    fs_info->devfsd_info = kmalloc (sizeof *fs_info->devfsd_info,
 					    GFP_KERNEL);

fs/proc/array.c renamed from fs/proc/array.c~pgrp +1 -1

diff -puN fs/proc/array.c~pgrp fs/proc/array.c
@@ -344,7 +344,7 @@ int proc_pid_stat(struct task_struct *ta
 		task->comm,
 		state,
 		ppid,
-		task->pgrp,
+		task->group_leader->pgrp,
 		task->session,
 		tty_nr,
 		tty_pgrp,

kernel/exit.c renamed from kernel/exit.c~pgrp +10 ~~-10~~

diff -puN kernel/exit.c~pgrp kernel/exit.c
@@ -152,7 +152,7 @@ static int will_become_orphaned_pgrp(int
 				|| p->state >= TASK_ZOMBIE 
 				|| p->real_parent->pid == 1)
 			continue;
-		if (p->real_parent->pgrp != pgrp
+		if (p->real_parent->group_leader->pgrp != pgrp
 			    && p->real_parent->session == p->session) {
 			ret = 0;
 			break;

@@ -247,9 +247,9 @@ void __set_special_pids(pid_t session, p
 		curr->session = session;
 		attach_pid(curr, PIDTYPE_SID, session);
 	}
-	if (curr->pgrp != pgrp) {
+	if (curr->group_leader->pgrp != pgrp) {
 		detach_pid(curr, PIDTYPE_PGID);
-		curr->pgrp = pgrp;
+		curr->group_leader->pgrp = pgrp;
 		attach_pid(curr, PIDTYPE_PGID, pgrp);
 	}
 }

@@ -618,12 +618,12 @@ static void exit_notify(struct task_stru
 	 
 	t = tsk->real_parent;
 	
-	if ((t->pgrp != tsk->pgrp) &&
+	if ((t->group_leader->pgrp != tsk->group_leader->pgrp) &&
 	    (t->session == tsk->session) &&
-	    will_become_orphaned_pgrp(tsk->pgrp, tsk) &&
-	    has_stopped_jobs(tsk->pgrp)) {
-		__kill_pg_info(SIGHUP, (void *)1, tsk->pgrp);
-		__kill_pg_info(SIGCONT, (void *)1, tsk->pgrp);
+	    will_become_orphaned_pgrp(tsk->group_leader->pgrp, tsk) &&
+	    has_stopped_jobs(tsk->group_leader->pgrp)) {
+		__kill_pg_info(SIGHUP, (void *)1, tsk->group_leader->pgrp);
+		__kill_pg_info(SIGCONT, (void *)1, tsk->group_leader->pgrp);
 	}
 
 	/* Let father know we died

@@ -813,10 +813,10 @@ static int eligible_child(pid_t pid, int
 		if (p->pid != pid)
 			return 0;
 	} else if (!pid) {
-		if (p->pgrp != current->pgrp)
+		if (p->group_leader->pgrp != current->group_leader->pgrp)
 			return 0;
 	} else if (pid != -1) {
-		if (p->pgrp != -pid)
+		if (p->group_leader->pgrp != -pid)
 			return 0;
 	}

kernel/pid.c renamed from kernel/pid.c~pgrp +1 -1

diff -puN kernel/pid.c~pgrp kernel/pid.c
@@ -250,7 +250,7 @@ void switch_exec_pids(task_t *leader, ta
 
 	attach_pid(thread, PIDTYPE_PID, thread->pid);
 	attach_pid(thread, PIDTYPE_TGID, thread->tgid);
-	attach_pid(thread, PIDTYPE_PGID, thread->pgrp);
+	attach_pid(thread, PIDTYPE_PGID, leader->pgrp);
 	attach_pid(thread, PIDTYPE_SID, thread->session);
 	list_add_tail(&thread->tasks, &init_task.tasks);

kernel/signal.c renamed from kernel/signal.c~pgrp +2 -2

diff -puN kernel/signal.c~pgrp kernel/signal.c
@@ -1139,7 +1139,7 @@ kill_proc_info(int sig, struct siginfo *
 static int kill_something_info(int sig, struct siginfo *info, int pid)
 {
 	if (!pid) {
-		return kill_pg_info(sig, info, current->pgrp);
+		return kill_pg_info(sig, info, current->group_leader->pgrp);
 	} else if (pid == -1) {
 		int retval = 0, count = 0;
 		struct task_struct * p;

@@ -1798,7 +1798,7 @@ relock:
 
 			/* signals can be posted during this window */
 
-			if (is_orphaned_pgrp(current->pgrp))
+			if (is_orphaned_pgrp(current->group_leader->pgrp))
 				goto relock;
 
 			spin_lock_irq(&current->sighand->siglock);

kernel/sys.c renamed from kernel/sys.c~pgrp +5 ~~-16~~

diff -puN kernel/sys.c~pgrp kernel/sys.c
@@ -290,7 +290,7 @@ asmlinkage long sys_setpriority(int whic
 			break;
 		case PRIO_PGRP:
 			if (!who)
-				who = current->pgrp;
+				who = current->group_leader->pgrp;
 			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid)
 				error = set_one_prio(p, niceval, error);
 			break;

@@ -346,7 +346,7 @@ asmlinkage long sys_getpriority(int whic
 			break;
 		case PRIO_PGRP:
 			if (!who)
-				who = current->pgrp;
+				who = current->group_leader->pgrp;
 			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid) {
 				niceval = 20 - task_nice(p);
 				if (niceval > retval)

@@ -985,17 +985,6 @@ ok_pgid:
 		attach_pid(p, PIDTYPE_PGID, pgid);
 	}
 
-	{
-		/* update all threads in thread group
-		   to new process group */
-		struct task_struct *p;
-		struct pid *pidp;
-		struct list_head *l;
-
-		for_each_task_pid(pid, PIDTYPE_TGID, p, l, pidp)
-			p->pgrp = pgid;
-	}
-
 	err = 0;
 out:
 	/* All paths lead to here, thus we are safe. -DaveM */

@@ -1006,7 +995,7 @@ out:
 asmlinkage long sys_getpgid(pid_t pid)
 {
 	if (!pid) {
-		return current->pgrp;
+		return current->group_leader->pgrp;
 	} else {
 		int retval;
 		struct task_struct *p;

@@ -1018,7 +1007,7 @@ asmlinkage long sys_getpgid(pid_t pid)
 		if (p) {
 			retval = security_task_getpgid(p);
 			if (!retval)
-				retval = p->pgrp;
+				retval = p->group_leader->pgrp;
 		}
 		read_unlock(&tasklist_lock);
 		return retval;

@@ -1028,7 +1017,7 @@ asmlinkage long sys_getpgid(pid_t pid)
 asmlinkage long sys_getpgrp(void)
 {
 	/* SMP - assuming writes are word atomic this is fine */
-	return current->pgrp;
+	return current->group_leader->pgrp;
 }
 
 asmlinkage long sys_getsid(pid_t pid)

Andrew Morton

2003-09-09 02:10 UTC #3 lore raw

Quoted 6 lines · Jeremy Fitzhardinge

Quoted 1 line · Andrew Morton
Using current->thread_group->foo sounds like a reasonable solution
Here's a patch to use group_leader->pgrp rather than just ->pgrp so that all threads in a group appear to have the same process group ID.

Just a quick maintainance thing:

Review

Quoted 2 lines

(quoted diff) +1 -1

-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(current->group_leader->pgrp))

Would it make sense to do this via

if (is_orphaned_pgrp(process_group(current)))

and to then rename task_struct.pgrp to something else, to pick up any missed conversions?

Jeremy Fitzhardinge

author

2003-09-09 02:16 UTC #4 lore raw

Quoted 2 lines · Andrew Morton

and to then rename task_struct.pgrp to something else, to pick up any missed conversions?

Probably a good idea. I was also thinking about renaming "group_leader" to something which tells you which kind of group its the leader of. Or at least commenting it.

Andrew Morton

2003-09-09 03:20 UTC #5 lore raw

Quoted 6 lines · Jeremy Fitzhardinge

Quoted 2 lines · Andrew Morton
and to then rename task_struct.pgrp to something else, to pick up any missed conversions?
Probably a good idea.

Caught a few:

drivers/char/tty_io.c: In function `tty_check_change':
drivers/char/tty_io.c:332: structure has no member named pgrp' drivers/char/tty_io.c:334: structure has no member named pgrp'

	if (is_orphaned_pgrp(current->pgrp))
		return -EIO;
	(void) kill_pg(current->pgrp,SIGTTOU,1);

should these be converted?

drivers/char/tty_io.c: In function `tty_open':
drivers/char/tty_io.c:1409: structure has no member named `pgrp'

		tty->pgrp = current->pgrp;

this one?

drivers/char/tty_io.c: In function `tiocsctty':
drivers/char/tty_io.c:1583: structure has no member named `pgrp'

        tty->pgrp = current->pgrp;

and this. I think so?

Ulrich Drepper

2003-09-09 03:55 UTC #6 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Andrew Morton

I think so?

Definitely. All occurrences have to be changed.

- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/XU762ijCOnn/RHQRAivlAJ44IyipfOulKbyIXFc2KtKNAChnAACcD9IP Di3DV6lM6+59ujhFwYK41mE=
=6Ofq
-----END PGP SIGNATURE-----

Andrew Morton

2003-09-09 04:17 UTC #7 lore raw

Quoted 2 lines · Ulrich Drepper

Definitely. All occurrences have to be changed.

OK. There were a few missing ones. Here's what I have, against test5. Do you have a test suite which would give us reasonable confidence?

Could you please grep for `__pgrp' and make sure that we got everything right there?

All other instances of foo->pgrp have been converted to process_group(foo), which is foo->group_leader->__pgrp.

Should setpgrp() in a thread affect the group leader? It does...

 arch/h8300/kernel/signal.c     |    2 +-
 arch/m68k/kernel/signal.c      |    2 +-
 arch/m68knommu/kernel/signal.c |    2 +-
 arch/mips/kernel/irixelf.c     |    2 +-
 arch/mips/kernel/irixsig.c     |    2 +-
 arch/mips/kernel/sysirix.c     |    4 ++--
 arch/sparc64/solaris/misc.c    |    4 ++--
 drivers/char/n_tty.c           |    6 +++---
 drivers/char/rocket.c          |    2 +-
 drivers/char/tty_io.c          |   10 +++++-----
 fs/autofs/autofs_i.h           |    2 +-
 fs/autofs/inode.c              |    2 +-
 fs/autofs/root.c               |    4 ++--
 fs/autofs4/autofs_i.h          |    2 +-
 fs/autofs4/inode.c             |    4 ++--
 fs/autofs4/root.c              |    4 ++--
 fs/binfmt_elf.c                |    4 ++--
 fs/coda/upcall.c               |    2 +-
 fs/devfs/base.c                |    6 +++---
 fs/proc/array.c                |    2 +-
 include/linux/sched.h          |    7 ++++++-
 kernel/exit.c                  |   24 ++++++++++++------------
 kernel/fork.c                  |    2 +-
 kernel/pid.c                   |    4 ++--
 kernel/signal.c                |    4 ++--
 kernel/sys.c                   |   17 +++++++++--------
 26 files changed, 66 insertions(+), 60 deletions(-)

kernel/sys.c renamed from kernel/sys.c~group_leader-rework +9 -8

diff -puN kernel/sys.c~group_leader-rework kernel/sys.c
@@ -290,7 +290,7 @@ asmlinkage long sys_setpriority(int whic
 			break;
 		case PRIO_PGRP:
 			if (!who)
-				who = current->pgrp;
+				who = process_group(current);
 			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid)
 				error = set_one_prio(p, niceval, error);
 			break;

@@ -346,7 +346,7 @@ asmlinkage long sys_getpriority(int whic
 			break;
 		case PRIO_PGRP:
 			if (!who)
-				who = current->pgrp;
+				who = process_group(current);
 			for_each_task_pid(who, PIDTYPE_PGID, p, l, pid) {
 				niceval = 20 - task_nice(p);
 				if (niceval > retval)

@@ -979,11 +979,12 @@ ok_pgid:
 	if (err)
 		goto out;
 
-	if (p->pgrp != pgid) {
+	if (process_group(p) != pgid) {
 		detach_pid(p, PIDTYPE_PGID);
-		p->pgrp = pgid;
+		p->group_leader->__pgrp = pgid;
 		attach_pid(p, PIDTYPE_PGID, pgid);
 	}
+
 	err = 0;
 out:
 	/* All paths lead to here, thus we are safe. -DaveM */

@@ -994,7 +995,7 @@ out:
 asmlinkage long sys_getpgid(pid_t pid)
 {
 	if (!pid) {
-		return current->pgrp;
+		return process_group(current);
 	} else {
 		int retval;
 		struct task_struct *p;

@@ -1006,7 +1007,7 @@ asmlinkage long sys_getpgid(pid_t pid)
 		if (p) {
 			retval = security_task_getpgid(p);
 			if (!retval)
-				retval = p->pgrp;
+				retval = process_group(p);
 		}
 		read_unlock(&tasklist_lock);
 		return retval;

@@ -1016,7 +1017,7 @@ asmlinkage long sys_getpgid(pid_t pid)
 asmlinkage long sys_getpgrp(void)
 {
 	/* SMP - assuming writes are word atomic this is fine */
-	return current->pgrp;
+	return process_group(current);
 }
 
 asmlinkage long sys_getsid(pid_t pid)

@@ -1059,7 +1060,7 @@ asmlinkage long sys_setsid(void)
 	__set_special_pids(current->pid, current->pid);
 	current->tty = NULL;
 	current->tty_old_pgrp = 0;
-	err = current->pgrp;
+	err = process_group(current);
 out:
 	write_unlock_irq(&tasklist_lock);
 	return err;

arch/h8300/kernel/signal.c renamed from arch/h8300/kernel/signal.c~group_leader-rework +1 -1

diff -puN arch/h8300/kernel/signal.c~group_leader-rework arch/h8300/kernel/signal.c
@@ -593,7 +593,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(process_group(current)))
 					continue;
 				/* FALLTHRU */

arch/m68k/kernel/signal.c renamed from arch/m68k/kernel/signal.c~group_leader-rework +1 -1

diff -puN arch/m68k/kernel/signal.c~group_leader-rework arch/m68k/kernel/signal.c
@@ -1082,7 +1082,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(process_group(current)))
 					continue;
 				/* FALLTHRU */

arch/m68knommu/kernel/signal.c renamed from arch/m68knommu/kernel/signal.c~group_leader-rework +1 -1

diff -puN arch/m68knommu/kernel/signal.c~group_leader-rework arch/m68knommu/kernel/signal.c
@@ -841,7 +841,7 @@ asmlinkage int do_signal(sigset_t *oldse
 				continue;
 
 			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(current->pgrp))
+				if (is_orphaned_pgrp(process_group(current)))
 					continue;
 				/* FALLTHRU */

arch/mips/kernel/irixelf.c renamed from arch/mips/kernel/irixelf.c~group_leader-rework +1 -1

diff -puN arch/mips/kernel/irixelf.c~group_leader-rework arch/mips/kernel/irixelf.c
@@ -1129,7 +1129,7 @@ static int irix_core_dump(long signr, st
 	prstatus.pr_sighold = current->blocked.sig[0];
 	psinfo.pr_pid = prstatus.pr_pid = current->pid;
 	psinfo.pr_ppid = prstatus.pr_ppid = current->parent->pid;
-	psinfo.pr_pgrp = prstatus.pr_pgrp = current->pgrp;
+	psinfo.pr_pgrp = prstatus.pr_pgrp = process_group(current);
 	psinfo.pr_sid = prstatus.pr_sid = current->session;
 	prstatus.pr_utime.tv_sec = CT_TO_SECS(current->utime);
 	prstatus.pr_utime.tv_usec = CT_TO_USECS(current->utime);

arch/mips/kernel/irixsig.c renamed from arch/mips/kernel/irixsig.c~group_leader-rework +1 -1

diff -puN arch/mips/kernel/irixsig.c~group_leader-rework arch/mips/kernel/irixsig.c
@@ -582,7 +582,7 @@ repeat:
 		p = list_entry(_p,struct task_struct,sibling);
 		if ((type == P_PID) && p->pid != pid)
 			continue;
-		if ((type == P_PGID) && p->pgrp != pid)
+		if ((type == P_PGID) && process_group(p) != pid)
 			continue;
 		if ((p->exit_signal != SIGCHLD))
 			continue;

arch/mips/kernel/sysirix.c renamed from arch/mips/kernel/sysirix.c~group_leader-rework +2 -2

diff -puN arch/mips/kernel/sysirix.c~group_leader-rework arch/mips/kernel/sysirix.c
@@ -803,11 +803,11 @@ asmlinkage int irix_setpgrp(int flags)
 	printk("[%s:%d] setpgrp(%d) ", current->comm, current->pid, flags);
 #endif
 	if(!flags)
-		error = current->pgrp;
+		error = process_group(current);
 	else
 		error = sys_setsid();
 #ifdef DEBUG_PROCGRPS
-	printk("returning %d\n", current->pgrp);
+	printk("returning %d\n", process_group(current));
 #endif
 
 	return error;

arch/sparc64/solaris/misc.c renamed from arch/sparc64/solaris/misc.c~group_leader-rework +2 -2

diff -puN arch/sparc64/solaris/misc.c~group_leader-rework arch/sparc64/solaris/misc.c
@@ -392,7 +392,7 @@ asmlinkage int solaris_procids(int cmd, 
 	
 	switch (cmd) {
 	case 0: /* getpgrp */
-		return current->pgrp;
+		return process_group(current);
 	case 1: /* setpgrp */
 		{
 			int (*sys_setpgid)(pid_t,pid_t) =

@@ -403,7 +403,7 @@ asmlinkage int solaris_procids(int cmd, 
 			ret = sys_setpgid(0, 0);
 			if (ret) return ret;
 			current->tty = NULL;
-			return current->pgrp;
+			return process_group(current);
 		}
 	case 2: /* getsid */
 		{

drivers/char/n_tty.c renamed from drivers/char/n_tty.c~group_leader-rework +3 -3

diff -puN drivers/char/n_tty.c~group_leader-rework drivers/char/n_tty.c
@@ -977,11 +977,11 @@ do_it_again:
 	if (file->f_op->write != redirected_tty_write && current->tty == tty) {
 		if (tty->pgrp <= 0)
 			printk("read_chan: tty->pgrp <= 0!\n");
-		else if (current->pgrp != tty->pgrp) {
+		else if (process_group(current) != tty->pgrp) {
 			if (is_ignored(SIGTTIN) ||
-			    is_orphaned_pgrp(current->pgrp))
+			    is_orphaned_pgrp(process_group(current)))
 				return -EIO;
-			kill_pg(current->pgrp, SIGTTIN, 1);
+			kill_pg(process_group(current), SIGTTIN, 1);
 			return -ERESTARTSYS;
 		}
 	}

drivers/char/rocket.c renamed from drivers/char/rocket.c~group_leader-rework +1 -1

diff -puN drivers/char/rocket.c~group_leader-rework drivers/char/rocket.c
@@ -956,7 +956,7 @@ static int rp_open(struct tty_struct *tt
 	 * Info->count is now 1; so it's safe to sleep now.
 	 */
 	info->session = current->session;
-	info->pgrp = current->pgrp;
+	info->pgrp = process_group(current);
 
 	if ((info->flags & ROCKET_INITIALIZED) == 0) {
 		cp = &info->channel;

drivers/char/tty_io.c renamed from drivers/char/tty_io.c~group_leader-rework +5 -5

diff -puN drivers/char/tty_io.c~group_leader-rework drivers/char/tty_io.c
@@ -325,13 +325,13 @@ int tty_check_change(struct tty_struct *
 		printk(KERN_WARNING "tty_check_change: tty->pgrp <= 0!\n");
 		return 0;
 	}
-	if (current->pgrp == tty->pgrp)
+	if (process_group(current) == tty->pgrp)
 		return 0;
 	if (is_ignored(SIGTTOU))
 		return 0;
-	if (is_orphaned_pgrp(current->pgrp))
+	if (is_orphaned_pgrp(process_group(current)))
 		return -EIO;
-	(void) kill_pg(current->pgrp,SIGTTOU,1);
+	(void) kill_pg(process_group(current), SIGTTOU, 1);
 	return -ERESTARTSYS;
 }

@@ -1406,7 +1406,7 @@ got_driver:
 		task_unlock(current);
 		current->tty_old_pgrp = 0;
 		tty->session = current->session;
-		tty->pgrp = current->pgrp;
+		tty->pgrp = process_group(current);
 	}
 	return 0;
 }

@@ -1580,7 +1580,7 @@ static int tiocsctty(struct tty_struct *
 	task_unlock(current);
 	current->tty_old_pgrp = 0;
 	tty->session = current->session;
-	tty->pgrp = current->pgrp;
+	tty->pgrp = process_group(current);
 	return 0;
 }

fs/autofs4/autofs_i.h renamed from fs/autofs4/autofs_i.h~group_leader-rework +1 -1

diff -puN fs/autofs4/autofs_i.h~group_leader-rework fs/autofs4/autofs_i.h
@@ -113,7 +113,7 @@ static inline struct autofs_info *autofs
    filesystem without "magic".) */
 
 static inline int autofs4_oz_mode(struct autofs_sb_info *sbi) {
-	return sbi->catatonic || current->pgrp == sbi->oz_pgrp;
+	return sbi->catatonic || process_group(current) == sbi->oz_pgrp;
 }
 
 /* Does a dentry have some pending activity? */

fs/autofs4/inode.c renamed from fs/autofs4/inode.c~group_leader-rework +2 -2

diff -puN fs/autofs4/inode.c~group_leader-rework fs/autofs4/inode.c
@@ -101,7 +101,7 @@ static int parse_options(char *options, 
 	
 	*uid = current->uid;
 	*gid = current->gid;
-	*pgrp = current->pgrp;
+	*pgrp = process_group(current);
 
 	*minproto = AUTOFS_MIN_PROTO_VERSION;
 	*maxproto = AUTOFS_MAX_PROTO_VERSION;

@@ -192,7 +192,7 @@ int autofs4_fill_super(struct super_bloc
 	sbi->magic = AUTOFS_SBI_MAGIC;
 	sbi->catatonic = 0;
 	sbi->exp_timeout = 0;
-	sbi->oz_pgrp = current->pgrp;
+	sbi->oz_pgrp = process_group(current);
 	sbi->sb = s;
 	sbi->version = 0;
 	sbi->queues = NULL;

fs/autofs4/root.c renamed from fs/autofs4/root.c~group_leader-rework +2 -2

diff -puN fs/autofs4/root.c~group_leader-rework fs/autofs4/root.c
@@ -255,7 +255,7 @@ static struct dentry *autofs4_root_looku
 	lock_kernel();
 	oz_mode = autofs4_oz_mode(sbi);
 	DPRINTK(("autofs_lookup: pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
-		 current->pid, current->pgrp, sbi->catatonic, oz_mode));
+		 current->pid, process_group(current), sbi->catatonic, oz_mode));
 
 	/*
 	 * Mark the dentry incomplete, but add it. This is needed so

@@ -518,7 +518,7 @@ static int autofs4_root_ioctl(struct ino
 	struct autofs_sb_info *sbi = autofs4_sbi(inode->i_sb);
 
 	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",
-		 cmd,arg,sbi,current->pgrp));
+		 cmd,arg,sbi,process_group(current)));
 
 	if ( _IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) ||
 	     _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT )

fs/autofs/autofs_i.h renamed from fs/autofs/autofs_i.h~group_leader-rework +1 -1

diff -puN fs/autofs/autofs_i.h~group_leader-rework fs/autofs/autofs_i.h
@@ -123,7 +123,7 @@ static inline struct autofs_sb_info *aut
    filesystem without "magic".) */
 
 static inline int autofs_oz_mode(struct autofs_sb_info *sbi) {
-	return sbi->catatonic || current->pgrp == sbi->oz_pgrp;
+	return sbi->catatonic || process_group(current) == sbi->oz_pgrp;
 }
 
 /* Hash operations */

fs/autofs/inode.c renamed from fs/autofs/inode.c~group_leader-rework +1 -1

diff -puN fs/autofs/inode.c~group_leader-rework fs/autofs/inode.c
@@ -51,7 +51,7 @@ static int parse_options(char *options, 
 	
 	*uid = current->uid;
 	*gid = current->gid;
-	*pgrp = current->pgrp;
+	*pgrp = process_group(current);
 
 	*minproto = *maxproto = AUTOFS_PROTO_VERSION;

fs/autofs/root.c renamed from fs/autofs/root.c~group_leader-rework +2 -2

diff -puN fs/autofs/root.c~group_leader-rework fs/autofs/root.c
@@ -213,7 +213,7 @@ static struct dentry *autofs_root_lookup
 
 	oz_mode = autofs_oz_mode(sbi);
 	DPRINTK(("autofs_lookup: pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
-		 current->pid, current->pgrp, sbi->catatonic, oz_mode));
+		 current->pid, process_group(current), sbi->catatonic, oz_mode));
 
 	/*
 	 * Mark the dentry incomplete, but add it. This is needed so

@@ -527,7 +527,7 @@ static int autofs_root_ioctl(struct inod
 {
 	struct autofs_sb_info *sbi = autofs_sbi(inode->i_sb);
 
-	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",cmd,arg,sbi,current->pgrp));
+	DPRINTK(("autofs_ioctl: cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n",cmd,arg,sbi,process_group(current)));
 
 	if ( _IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) ||
 	     _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT )

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~group_leader-rework +2 -2

diff -puN fs/binfmt_elf.c~group_leader-rework fs/binfmt_elf.c
@@ -1076,7 +1076,7 @@ static void fill_prstatus(struct elf_prs
 	prstatus->pr_sighold = p->blocked.sig[0];
 	prstatus->pr_pid = p->pid;
 	prstatus->pr_ppid = p->parent->pid;
-	prstatus->pr_pgrp = p->pgrp;
+	prstatus->pr_pgrp = process_group(p);
 	prstatus->pr_sid = p->session;
 	jiffies_to_timeval(p->utime, &prstatus->pr_utime);
 	jiffies_to_timeval(p->stime, &prstatus->pr_stime);

@@ -1104,7 +1104,7 @@ static void fill_psinfo(struct elf_prpsi
 
 	psinfo->pr_pid = p->pid;
 	psinfo->pr_ppid = p->parent->pid;
-	psinfo->pr_pgrp = p->pgrp;
+	psinfo->pr_pgrp = process_group(p);
 	psinfo->pr_sid = p->session;
 
 	i = p->state ? ffz(~p->state) + 1 : 0;

fs/coda/upcall.c renamed from fs/coda/upcall.c~group_leader-rework +1 -1

diff -puN fs/coda/upcall.c~group_leader-rework fs/coda/upcall.c
@@ -54,7 +54,7 @@ static void *alloc_upcall(int opcode, in
 
         inp->ih.opcode = opcode;
 	inp->ih.pid = current->pid;
-	inp->ih.pgid = current->pgrp;
+	inp->ih.pgid = process_group(current);
 	coda_load_creds(&(inp->ih.cred));
 
 	return (void*)inp;

fs/devfs/base.c renamed from fs/devfs/base.c~group_leader-rework +3 -3

diff -puN fs/devfs/base.c~group_leader-rework fs/devfs/base.c
@@ -1334,7 +1334,7 @@ static int is_devfsd_or_child (struct fs
     struct task_struct *p = current;
 
     if (p == fs_info->devfsd_task) return (TRUE);
-    if (p->pgrp == fs_info->devfsd_pgrp) return (TRUE);
+    if (process_group(p) == fs_info->devfsd_pgrp) return (TRUE);
     read_lock(&tasklist_lock);
     for ( ; p != &init_task; p = p->real_parent)
     {

@@ -2744,8 +2744,8 @@ static int devfsd_ioctl (struct inode *i
 	    }
 	    fs_info->devfsd_task = current;
 	    spin_unlock (&lock);
-	    fs_info->devfsd_pgrp = (current->pgrp == current->pid) ?
-		current->pgrp : 0;
+	    fs_info->devfsd_pgrp = (process_group(current) == current->pid) ?
+		process_group(current) : 0;
 	    fs_info->devfsd_file = file;
 	    fs_info->devfsd_info = kmalloc (sizeof *fs_info->devfsd_info,
 					    GFP_KERNEL);

fs/proc/array.c renamed from fs/proc/array.c~group_leader-rework +1 -1

diff -puN fs/proc/array.c~group_leader-rework fs/proc/array.c
@@ -341,7 +341,7 @@ int proc_pid_stat(struct task_struct *ta
 		task->comm,
 		state,
 		ppid,
-		task->pgrp,
+		process_group(task),
 		task->session,
 		tty_nr,
 		tty_pgrp,

kernel/exit.c renamed from kernel/exit.c~group_leader-rework +12 ~~-12~~

diff -puN kernel/exit.c~group_leader-rework kernel/exit.c
@@ -152,7 +152,7 @@ static int will_become_orphaned_pgrp(int
 				|| p->state >= TASK_ZOMBIE 
 				|| p->real_parent->pid == 1)
 			continue;
-		if (p->real_parent->pgrp != pgrp
+		if (process_group(p->real_parent) != pgrp
 			    && p->real_parent->session == p->session) {
 			ret = 0;
 			break;

@@ -247,9 +247,9 @@ void __set_special_pids(pid_t session, p
 		curr->session = session;
 		attach_pid(curr, PIDTYPE_SID, session);
 	}
-	if (curr->pgrp != pgrp) {
+	if (process_group(curr) != pgrp) {
 		detach_pid(curr, PIDTYPE_PGID);
-		curr->pgrp = pgrp;
+		curr->group_leader->__pgrp = pgrp;
 		attach_pid(curr, PIDTYPE_PGID, pgrp);
 	}
 }

@@ -508,9 +508,9 @@ static inline void reparent_thread(task_
 	 * than we are, and it was the only connection
 	 * outside, so the child pgrp is now orphaned.
 	 */
-	if ((p->pgrp != father->pgrp) &&
+	if ((process_group(p) != process_group(father)) &&
 	    (p->session == father->session)) {
-		int pgrp = p->pgrp;
+		int pgrp = process_group(p);
 
 		if (will_become_orphaned_pgrp(pgrp, NULL) && has_stopped_jobs(pgrp)) {
 			__kill_pg_info(SIGHUP, (void *)1, pgrp);

@@ -618,12 +618,12 @@ static void exit_notify(struct task_stru
 	 
 	t = tsk->real_parent;
 	
-	if ((t->pgrp != tsk->pgrp) &&
+	if ((process_group(t) != process_group(tsk)) &&
 	    (t->session == tsk->session) &&
-	    will_become_orphaned_pgrp(tsk->pgrp, tsk) &&
-	    has_stopped_jobs(tsk->pgrp)) {
-		__kill_pg_info(SIGHUP, (void *)1, tsk->pgrp);
-		__kill_pg_info(SIGCONT, (void *)1, tsk->pgrp);
+	    will_become_orphaned_pgrp(process_group(tsk), tsk) &&
+	    has_stopped_jobs(process_group(tsk))) {
+		__kill_pg_info(SIGHUP, (void *)1, process_group(tsk));
+		__kill_pg_info(SIGCONT, (void *)1, process_group(tsk));
 	}
 
 	/* Let father know we died

@@ -813,10 +813,10 @@ static int eligible_child(pid_t pid, int
 		if (p->pid != pid)
 			return 0;
 	} else if (!pid) {
-		if (p->pgrp != current->pgrp)
+		if (process_group(p) != process_group(current))
 			return 0;
 	} else if (pid != -1) {
-		if (p->pgrp != -pid)
+		if (process_group(p) != -pid)
 			return 0;
 	}

kernel/pid.c renamed from kernel/pid.c~group_leader-rework +2 -2

diff -puN kernel/pid.c~group_leader-rework kernel/pid.c
@@ -250,13 +250,13 @@ void switch_exec_pids(task_t *leader, ta
 
 	attach_pid(thread, PIDTYPE_PID, thread->pid);
 	attach_pid(thread, PIDTYPE_TGID, thread->tgid);
-	attach_pid(thread, PIDTYPE_PGID, thread->pgrp);
+	attach_pid(thread, PIDTYPE_PGID, leader->__pgrp);
 	attach_pid(thread, PIDTYPE_SID, thread->session);
 	list_add_tail(&thread->tasks, &init_task.tasks);
 
 	attach_pid(leader, PIDTYPE_PID, leader->pid);
 	attach_pid(leader, PIDTYPE_TGID, leader->tgid);
-	attach_pid(leader, PIDTYPE_PGID, leader->pgrp);
+	attach_pid(leader, PIDTYPE_PGID, leader->__pgrp);
 	attach_pid(leader, PIDTYPE_SID, leader->session);
 }

kernel/signal.c renamed from kernel/signal.c~group_leader-rework +2 -2

diff -puN kernel/signal.c~group_leader-rework kernel/signal.c
@@ -1139,7 +1139,7 @@ kill_proc_info(int sig, struct siginfo *
 static int kill_something_info(int sig, struct siginfo *info, int pid)
 {
 	if (!pid) {
-		return kill_pg_info(sig, info, current->pgrp);
+		return kill_pg_info(sig, info, process_group(current));
 	} else if (pid == -1) {
 		int retval = 0, count = 0;
 		struct task_struct * p;

@@ -1798,7 +1798,7 @@ relock:
 
 			/* signals can be posted during this window */
 
-			if (is_orphaned_pgrp(current->pgrp))
+			if (is_orphaned_pgrp(process_group(current)))
 				goto relock;
 
 			spin_lock_irq(&current->sighand->siglock);

include/linux/sched.h renamed from include/linux/sched.h~group_leader-rework +6 -1

diff -puN include/linux/sched.h~group_leader-rework include/linux/sched.h
@@ -360,7 +360,7 @@ struct task_struct {
 	unsigned long personality;
 	int did_exec:1;
 	pid_t pid;
-	pid_t pgrp;
+	pid_t __pgrp;		/* Accessed via process_group() */
 	pid_t tty_old_pgrp;
 	pid_t session;
 	pid_t tgid;

@@ -461,6 +461,11 @@ struct task_struct {
 	siginfo_t *last_siginfo; /* For ptrace use.  */
 };
 
+static inline pid_t process_group(struct task_struct *tsk)
+{
+	return tsk->group_leader->__pgrp;
+}
+
 extern void __put_task_struct(struct task_struct *tsk);
 #define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)
 #define put_task_struct(tsk) \

kernel/fork.c renamed from kernel/fork.c~group_leader-rework +1 -1

diff -puN kernel/fork.c~group_leader-rework kernel/fork.c
@@ -1004,7 +1004,7 @@ struct task_struct *copy_process(unsigne
 	attach_pid(p, PIDTYPE_PID, p->pid);
 	if (thread_group_leader(p)) {
 		attach_pid(p, PIDTYPE_TGID, p->tgid);
-		attach_pid(p, PIDTYPE_PGID, p->pgrp);
+		attach_pid(p, PIDTYPE_PGID, process_group(p));
 		attach_pid(p, PIDTYPE_SID, p->session);
 		if (p->pid)
 			__get_cpu_var(process_counts)++;

Ulrich Drepper

2003-09-09 04:39 UTC #8 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 2 lines · Andrew Morton

OK. There were a few missing ones. Here's what I have, against test5. Do you have a test suite which would give us reasonable confidence?

Nope, I don't.

Why not leave the element renamed for a test release or two. So the nice people who do compile checks on all the drivers will find the missing changes.

Quoted 1 line

Should setpgrp() in a thread affect the group leader? It does...

Yes. This the existence of a thread group leader different from current
means using the POSIX thread "process" semantics this is the case. All
the thread together have only one "process group" ("process" again in the POSIX sense).

iD8DBQE/XVlG2ijCOnn/RHQRAnZnAJ0SaF7LUcbY/8XKxBnwYzW5pmpeYACglnWh 55qtBiGdqNyl6sAwR6OCPXg=
=5oJh
-----END PGP SIGNATURE-----

Jeremy Fitzhardinge

author

2003-09-09 05:56 UTC #9 lore raw

In reply to Andrew Morton

Quoted 4 lines · Andrew Morton

All other instances of foo->pgrp have been converted to process_group(foo), which is foo->group_leader->__pgrp.
Should setpgrp() in a thread affect the group leader? It does...

Eh? sys_setpgid() fails with EINVAL if another thread tries to change the process group. Oh, I see. setpgrp() is a wrapper which passes pid=0 to setpgid; sys_setpgid uses current->pid rather than current->tgid, so it always uses the thread group leader's ID. Looks OK to me.

Roland McGrath

2003-09-09 06:24 UTC #10 lore raw

In reply to Andrew Morton

Quoted 6 lines

Would it make sense to do this via
if (is_orphaned_pgrp(process_group(current)))
and to then rename task_struct.pgrp to something else, to pick up any missed conversions?

I am wholly in favor of this. I think what we really want to do is move pgrp into signal_struct, and that would be painless after the macroization.

Thanks,
Roland