Skip to contentSkip to conversation

multithreaded coredumps for elf exeecutables

PATCH In review Vamsi Krishna S . 5 participants 15 messages Mar 15, 2002 – Mar 22, 2002

Subsystems (10)
Memory management VFS USB Kbuild +6 more subsystems
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 multithreaded coredumps for elf exeecutables

Here is a kernel patch to support multithreaded coredumps being worked on by Mark Gross (Intel, [email protected]) and
Vamsi Krishna (IBM, [email protected]).

Multi-threaded core dump patch for 2.4.17:
- multithreaded coredump functionality is enabled by a new sysctl core_dumps_threads. (0 = off, 1 = on).

- Core dump is started by the first thread which gets the signal

- Threads are located by walking the entire task list looking for tasks with matching mm as that seems to be the only reliable way to locate other threads of a given task. In fact, IMO this is the only way until all user space libraries migrate to using thread groups provides by linux kernel (CLONE_THREAD).

- Other threads are prevented from executing while core dump is in progress to improve the accuracy of the dumps. This is done without changing the state of the task. We set cpus_allowed in task struct to be 0 to stop a task from being scheduled and reset it to -1 for resume execution. This has the advantage to not depending on user space at all for correct functioning. IMO sending SIGSTOP to stop other threads does not work if the process is being run under a debugger. The only possible issue with using cpus_allowed is that we could lose task affinities once a core dump is taken. However, this is not a big deal as the task is going to die anyway fairly soon, which is why the dump was taken in the first place.

- Support of SSE registers in the core dump

- Code cleanups/reorg - breakup into smaller functions. Main function elf_core_dump() reorganized/cleaned up by moving filling up of elfhdr, prstatus, psinfo and notes to separate functions to make this very long function a little bit more readable and to reuse some code with the function dumping status of other threads.

- Easy to port to other architectures. It just needs ELF_CORE_COPY_TASK_REG - to copy task specific registers ELF_CORE_COPY_FPREGS - to copy floating point registers and ELF_CORE_COPY_XFPREGS - to copy extended fp registers(SSE) if present ELF_CORE_SYNC - to sync up fpu status of other processors in SMP systems if needed by a particular architecture. Read the patch for more details.

- We started with the tcore patches by John Jones and Jason Villarreal as base which were then heavily reworked. This patch is entirely different from theirs in pretty much all aspects.

Current TODO list:
- May be remove reschedule_other_cpus in suspend_other_threads and do this as part of ELF_CORE_SYNC. Rescheduling the other CPUs the way the current patch work may be over kill for accurate core files. Any thoughts?
- Port to 2.5.x, specially the logic to stop other threads from executing while dumping is in progress.
- Make the loop looking for other threads a little shorter by counting the number of tasks found and breaking out of for_each_task loop when it is equal to current->mm->mm_users.

Some usage notes on this patch:
GDB 5.1 works with the core files produced, but only for Red Hat 7.2, and only if the /lib/i686/libpthread.so library is hidden. It turns out that for IA32 RedHat, that there exists 2 libpthread.so files. If the /lib/i686/libpthread.so is loaded then the gdb post mortem debug will not work. We don't understand what's going on here, but its real. Hide the /lib/i686/libpthread.so such that the /lib/libpthread.so gets loaded at debug time, and then debugger will work with the core file. Any insights into this is very much welcome. This behavior is very mysterious to us.

Thanks to Bharata B Rao(IBM) for helping with capturing FPU registers and testing and Suparna Bhattacharya(IBM) for design discussions.

Thanks to Tony Luck (Intel) and Jun Nakajima (Intel) for helping with the review and design of the suspend_other_threads implementation.

This is currently i386 only, it has been unit tested on 1P-P4, 2P-P4, and 4P-PIII systems. I haven't seen any failures so far, YMMV.

The patch is against kernel version 2.4.17. We will port this to latest versions of the kernel if there is any interest.

Regards.. Vamsi.

Vamsi Krishna S.
Linux Technology Center,
IBM Software Lab, Bangalore.
Ph: +91 80 5044959
Internet: [email protected]

-- patch here--

arch/i386/kernel/i387.c renamed from usr/src/2417-pure/arch/i386/kernel/i387.c +39 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/arch/i386/kernel/i387.c 2417-tcore/arch/i386/kernel/i387.c
@@ -520,3 +520,42 @@
 
 	return fpvalid;
 }
+
+int dump_task_fpu( struct task_struct *tsk, struct user_i387_struct *fpu )
+{
+        int fpvalid;
+
+        fpvalid = tsk->used_math;
+        if ( fpvalid ) {
+                if (tsk == current) unlazy_fpu( tsk );
+                if ( cpu_has_fxsr ) {
+                        copy_fpu_fxsave( tsk, fpu );
+                } else {
+                        copy_fpu_fsave( tsk, fpu );
+                }
+        }
+
+        return fpvalid;
+}
+
+int dump_task_extended_fpu( struct task_struct *tsk, struct user_fxsr_struct *fpu )
+{
+        int fpvalid;
+
+        fpvalid = tsk->used_math && cpu_has_fxsr;
+        if ( fpvalid ) {
+                if (tsk == current) unlazy_fpu( tsk );
+                memcpy( fpu, &tsk->thread.i387.fxsave,
+                        sizeof(struct user_fxsr_struct) );
+        }
+
+        return fpvalid;
+}
+
+#ifdef CONFIG_SMP
+void dump_smp_unlazy_fpu(void)
+{
+	unlazy_fpu(current);
+	return;
+}
+#endif
arch/i386/kernel/process.c renamed from usr/src/2417-pure/arch/i386/kernel/process.c +13 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/arch/i386/kernel/process.c 2417-tcore/arch/i386/kernel/process.c
@@ -642,6 +642,19 @@
 	dump->u_fpvalid = dump_fpu (regs, &dump->i387);
 }
 
+/* 
+ * Capture the user space registers if the task is not running (in user space)
+ */
+int dump_task_regs(struct task_struct *tsk, struct pt_regs *regs)
+{
+	*regs = *(struct pt_regs *)((unsigned long)tsk + THREAD_SIZE - sizeof(struct pt_regs));
+	regs->xcs &= 0xffff;
+	regs->xds &= 0xffff;
+	regs->xes &= 0xffff;
+	regs->xss &= 0xffff;
+	return 1;
+}
+
 /*
  * This special macro can be used to load a debugging register
  */
fs/binfmt_elf.c renamed from usr/src/2417-pure/fs/binfmt_elf.c +337 -97

516 lines not on this page · raw message

include/asm-i386/elf.h renamed from usr/src/2417-pure/include/asm-i386/elf.h +12 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/include/asm-i386/elf.h 2417-tcore/include/asm-i386/elf.h
@@ -99,6 +99,18 @@
 
 #ifdef __KERNEL__
 #define SET_PERSONALITY(ex, ibcs2) set_personality((ibcs2)?PER_SVR4:PER_LINUX)
+
+extern int dump_task_regs (struct task_struct *, struct pt_regs *);
+extern int dump_task_fpu (struct task_struct *, struct user_i387_struct *);
+extern int dump_task_extended_fpu (struct task_struct *, struct user_fxsr_struct *);
+
+#define ELF_CORE_COPY_TASK_REGS(tsk, pt_regs) dump_task_regs(tsk, pt_regs)
+#define ELF_CORE_COPY_FPREGS(tsk, elf_fpregs) dump_task_fpu(tsk, elf_fpregs)
+#define ELF_CORE_COPY_XFPREGS(tsk, elf_xfpregs) dump_task_extended_fpu(tsk, elf_xfpregs)
+#ifdef CONFIG_SMP
+extern void dump_smp_unlazy_fpu(void);
+#define ELF_CORE_SYNC dump_smp_unlazy_fpu
 #endif
+#endif /* __KERNEL__ */
 
 #endif
include/linux/elf.h renamed from usr/src/2417-pure/include/linux/elf.h +2 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/include/linux/elf.h 2417-tcore/include/linux/elf.h
@@ -576,6 +576,8 @@
 #define NT_PRPSINFO	3
 #define NT_TASKSTRUCT	4
 #define NT_PRFPXREG	20
+#define NT_PRXFPREG     0x46e62b7f	/* note name must be "LINUX" as per GDB */
+					/* from gdb5.1/include/elf/common.h */
 
 /* Note header in a PT_NOTE section */
 typedef struct elf32_note {
include/linux/elfcore.h renamed from usr/src/2417-pure/include/linux/elfcore.h +52 -0

56 lines not on this page · raw message

include/linux/sched.h renamed from usr/src/2417-pure/include/linux/sched.h +4 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/include/linux/sched.h 2417-tcore/include/linux/sched.h
@@ -160,6 +160,10 @@
 extern int start_context_thread(void);
 extern int current_is_keventd(void);
 
+extern void reschedule_other_cpus(void);
+// forces all cpu's other than current to reschedule.  Needed for accurate core dumps.
+
+
 /*
  * The default fd array needs to be at least BITS_PER_LONG,
  * as this is the granularity returned by copy_fdset().
include/linux/sysctl.h renamed from usr/src/2417-pure/include/linux/sysctl.h +1 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/include/linux/sysctl.h 2417-tcore/include/linux/sysctl.h
@@ -87,6 +87,7 @@
 	KERN_CAP_BSET=14,	/* int: capability bounding set */
 	KERN_PANIC=15,		/* int: panic timeout */
 	KERN_REALROOTDEV=16,	/* real root device to mount after initrd */
+	KERN_CORE_DUMPS_THREADS=17, /* int: include status of others threads in dump */
 
 	KERN_SPARC_REBOOT=21,	/* reboot command on Sparc */
 	KERN_CTLALTDEL=22,	/* int: allow ctl-alt-del to reboot */
kernel/sched.c renamed from usr/src/2417-pure/kernel/sched.c +23 -1
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/kernel/sched.c 2417-tcore/kernel/sched.c
@@ -121,7 +121,7 @@
 #else
 
 #define idle_task(cpu) (&init_task)
-#define can_schedule(p,cpu) (1)
+#define can_schedule(p,cpu) ((p)->cpus_allowed)
 
 #endif
 
@@ -704,6 +704,28 @@
 	return;
 }
 
+/*
+ * needed for accurate core dumps of multi-threaded applications.
+ * see binfmt_elf.c for more information.
+ */
+void reschedule_other_cpus(void)
+{
+#ifdef CONFIG_SMP
+	int i, cpu;
+	struct task_struct *p;
+
+	for(i=0; i< smp_num_cpus; i++) {
+		cpu = cpu_logical_map(i);
+		p = cpu_curr(cpu);
+		if (p->processor != smp_processor_id()) {
+			p->need_resched = 1;
+			smp_send_reschedule(p->processor);
+		}
+	}
+#endif	
+	return;
+}
+
 /*
  * The core wakeup function.  Non-exclusive wakeups (nr_exclusive == 0) just wake everything
  * up.  If it's an exclusive wakeup (nr_exclusive == small +ve number) then we wake all the
kernel/sysctl.c renamed from usr/src/2417-pure/kernel/sysctl.c +3 -0
diff -urN -X /home/vamsi/dontdiff /usr/src/2417-pure/kernel/sysctl.c 2417-tcore/kernel/sysctl.c
@@ -49,6 +49,7 @@
 extern int max_queued_signals;
 extern int sysrq_enabled;
 extern int core_uses_pid;
+extern int core_dumps_threads;
 extern int cad_pid;
 
 /* this is needed for the proc_dointvec_minmax for [fs_]overflow UID and GID */
@@ -169,6 +170,8 @@
 	 0644, NULL, &proc_doutsstring, &sysctl_string},
 	{KERN_PANIC, "panic", &panic_timeout, sizeof(int),
 	 0644, NULL, &proc_dointvec},
+	{KERN_CORE_DUMPS_THREADS, "core_dumps_threads", &core_dumps_threads, sizeof(int),
+	 0644, NULL, &proc_dointvec},
 	{KERN_CORE_USES_PID, "core_uses_pid", &core_uses_pid, sizeof(int),
 	 0644, NULL, &proc_dointvec},
 	{KERN_TAINTED, "tainted", &tainted, sizeof(int),
#1 lore raw

Hi!

Quoted 8 lines

- Other threads are prevented from executing while core dump is in progress to improve the accuracy of the dumps. This is done without changing the state of the task. We set cpus_allowed in task struct to be 0 to stop a task from being scheduled and reset it to -1 for resume execution. This has the advantage to not depending on user space at all for correct functioning. IMO sending SIGSTOP to stop other threads does not work if the process is being run under a debugger. The only possible issue with using cpus_allowed is that

In swsusp patch, I had exactly the same problem. I created refrigerator(), and halfway send a signal....

Quoted 8 lines

+/*
+ * Suspend execution of other threads belonging to the same multithreaded process
+ * of current, ASAP.
+ *
+ * Sets the current->cpu_mask to the current cpu to avoid cpu migration durring the dump.
+ * This cpu will also be the only cpu the other threads will be allowed to run after
+ * coredump is completed. This seems to be needed to fix some SMP races. This still
+ * needs some more thought though this solution works.

What about

app has 5 threads. 1st dumps core, and starts setting cpus_allowed mask to thread 2. Meanwhile 3nd thread resets the mask back.

Pavel?

Signature · 2 lines
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.
#2 lore raw
In reply to Vamsi Krishna S .
Quoted 7 lines · Pavel Machek
Quoted 6 lines

+ *
+ * Sets the current->cpu_mask to the current cpu to avoid cpu migration durring the dump. + * This cpu will also be the only cpu the other threads will be allowed to run after + * coredump is completed. This seems to be needed to fix some SMP races. This still + * needs some more thought though this solution works.

Quoted 5 lines

What about

app has 5 threads. 1st dumps core, and starts setting cpus_allowed mask to thread 2. Meanwhile 3nd thread resets the mask back.

This patch was intended to prevent this from happening. I hope I didn't miss something.

The dumping thread doesn't proceed until the other CPU's have gotten into kernel mode and done 2 IPI's. One to reschedule the other cpu's and one to synchronize before exiting suspend_other_threads.

The way the IPI's are sent out by this patch, the other CPUs get 2 IPI's and execute at least one IRET, and hence at least one call to schedule, before the dumping process continues. This one call to schedule on each of the other cpu's is what's needed to get all possible related thread processes swapped out for the duration of the dump.

Unless the IPI's and associated IRET's get dropped by the system, that 3rd thread will not get a chance to touch the cpu_masks before the dumping process is finished taking its dump and resume_other_threads gets called. Because it will have been scheduled out.

--mgross

#3 lore raw
In reply to Vamsi Krishna S .

There is serialization at higher level. We take a write lock on current->mm->mmap_sem at the beginning of elf_core_dump function which is released just before leaving the function. So, if one thread enters elf_core_dump and starts dumping core, no other thread (same mm) of the same process can start dumping.

static int elf_core_dump(long signr, struct pt_regs * regs, struct file * file)

{
	...
	...
        /* now stop all vm operations */
        down_write(&current->mm->mmap_sem);
	...
	...
	...
        up_write(&current->mm->mmap_sem);
        return has_dumped;
}

Vamsi.

Signature · 5 lines
Vamsi Krishna S.
Linux Technology Center,
IBM Software Lab, Bangalore.
Ph: +91 80 5262355 Extn: 3959
Internet: [email protected]
Quoted 32 lines · Mark Gross
Quoted 7 lines · Pavel Machek
Quoted 6 lines

+ *
+ * Sets the current->cpu_mask to the current cpu to avoid cpu migration durring the dump. + * This cpu will also be the only cpu the other threads will be allowed to run after + * coredump is completed. This seems to be needed to fix some SMP races. This still + * needs some more thought though this solution works.

Quoted 5 lines

What about

app has 5 threads. 1st dumps core, and starts setting cpus_allowed mask to thread 2. Meanwhile 3nd thread resets the mask back.

This patch was intended to prevent this from happening. I hope I didn't miss something.

The dumping thread doesn't proceed until the other CPU's have gotten into kernel mode and done 2 IPI's. One to reschedule the other cpu's and one to synchronize before exiting suspend_other_threads.

The way the IPI's are sent out by this patch, the other CPUs get 2 IPI's and execute at least one IRET, and hence at least one call to schedule, before the dumping process continues. This one call to schedule on each of the other cpu's is what's needed to get all possible related thread processes swapped out for the duration of the dump.

Unless the IPI's and associated IRET's get dropped by the system, that 3rd thread will not get a chance to touch the cpu_masks before the dumping process is finished taking its dump and resume_other_threads gets called. Because it will have been scheduled out.

--mgross

#4 lore raw
In reply to Vamsi Krishna S .
Quoted 8 lines · Vamsi Krishna S .

There is serialization at higher level. We take a write lock on current->mm->mmap_sem at the beginning of elf_core_dump function which is released just before leaving the function. So, if one thread enters elf_core_dump and starts dumping core, no other thread (same mm) of the same process can start dumping.

static int elf_core_dump(long signr, struct pt_regs * regs, struct file * file)

Quoted 11 lines
{
	...
	...
        /* now stop all vm operations */
        down_write(&current->mm->mmap_sem);
	...
	...
	...
        up_write(&current->mm->mmap_sem);
        return has_dumped;
}

That's not a feature, it's a bug. You can't take the mmap_sem before collecting thread status; it will cause a deadlock on at least ia64, where some registers are collected from user memory.

(Thanks to Manfred Spraul for explaining that to me.)

Signature · 2 lines
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer
#5 lore raw
In reply to Vamsi Krishna S .

I've only JUST started on the Itanium version of this patch. In my initial testing, after hacking around some of the compilation issues, I do get a type of process freezing when attempting this. Could be this bug.

Thanks for the tip ;)

--mgross

Quoted 26 lines · Daniel Jacobowitz
Quoted 11 lines · Vamsi Krishna S .

 There is serialization at higher level. We take a write lock on current->mm->mmap_sem at the beginning of elf_core_dump function which is released just before leaving the function. So, if one thread enters elf_core_dump and starts dumping core, no other thread (same mm) of the same process can start dumping.

 static int elf_core_dump(long signr, struct pt_regs * regs, struct file * file) {
       ...
       ...

Quoted 8 lines

         /* now stop all vm operations */
         down_write(&current->mm->mmap_sem);
       ...
       ...
       ...
         up_write(&current->mm->mmap_sem);
         return has_dumped;
 }

That's not a feature, it's a bug.  You can't take the mmap_sem before collecting thread status; it will cause a deadlock on at least ia64, where some registers are collected from user memory.

(Thanks to Manfred Spraul for explaining that to me.)

#6 lore raw
In reply to Vamsi Krishna S .

Mark,

Does moving the down_write() to be after the registers of all threads are collected help? (This patch on top of our previous one)
--

fs/binfmt_elf.c renamed from fs/binfmt_elf.c.ori +13 -11
@@ -1289,10 +1289,6 @@
 	int dump_threads = 0;
 	int thread_status_size = 0;
 	
-	/* now stop all vm operations */
-	down_write(&current->mm->mmap_sem);
-	segs = current->mm->map_count;
-
  	if (atomic_read(&current->mm->mm_users) != 1) {
 		dump_threads = core_dumps_threads;
 	}
@@ -1337,6 +1333,19 @@
 		}
 	} /* End if(dump_threads) */
 
+	/*
+	 * This transfers the registers from regs into the standard
+	 * coredump arrangement, whatever that is. We need to do this
+	 * before acquiring mmap_sem as on some architectures (IA64)
+	 * we may need to access user pages to get register state.
+	 */
+	memset(&prstatus, 0, sizeof(prstatus));
+	elf_core_copy_regs(&prstatus.pr_reg, regs);
+
+	/* now stop all vm operations */
+	down_write(&current->mm->mmap_sem);
+	segs = current->mm->map_count;
+
 #ifdef DEBUG
 	printk("elf_core_dump: %d segs %lu limit\n", segs, limit);
 #endif
@@ -1358,16 +1367,9 @@
 	 * Set up the notes in similar form to SVR4 core dumps made
 	 * with info from their /proc.
 	 */
-	memset(&prstatus, 0, sizeof(prstatus));
 	fill_prstatus(&prstatus, current, signr);
 	fill_note(&notes[0], "CORE", NT_PRSTATUS, sizeof(prstatus), &prstatus);
 
-	/*
-	 * This transfers the registers from regs into the standard
-	 * coredump arrangement, whatever that is.
-	 */
-	elf_core_copy_regs(&prstatus.pr_reg, regs);
-
 #ifdef DEBUG
 	dump_regs("Passed in regs", (elf_greg_t *)regs);
 	dump_regs("prstatus regs", (elf_greg_t *)&prstatus.pr_reg);
Signature · 5 lines
Vamsi Krishna S.
Linux Technology Center,
IBM Software Lab, Bangalore.
Ph: +91 80 5262355 Extn: 3959
Internet: [email protected]
Quoted 25 lines · Mark Gross

I've only JUST started on the Itanium version of this patch. In my initial testing, after hacking around some of the compilation issues, I do get a type of process freezing when attempting this. Could be this bug.

Thanks for the tip ;)

--mgross

Quoted 14 lines · Daniel Jacobowitz
Quoted 7 lines · Vamsi Krishna S .

 There is serialization at higher level. We take a write lock on current->mm->mmap_sem at the beginning of elf_core_dump function which is released just before leaving the function. So, if one thread enters elf_core_dump and starts dumping core, no other thread (same mm) of the same process can start dumping.
 <snip>

That's not a feature, it's a bug.  You can't take the mmap_sem before collecting thread status; it will cause a deadlock on at least ia64, where some registers are collected from user memory.

(Thanks to Manfred Spraul for explaining that to me.)

#7 lore raw
In reply to Vamsi Krishna S .

Dan,

Thanks for pointing this out. I see that this change has now gone into 2.4.18 as well as 2.5.4. We would ensure that the down_write happens only after the registers of all threads are collected.

Coming back to the original point raised by Pavel, indeed there is nothing preventing external code (any other kernel modules) modifying the cpus_allowed field from under us. This could get worse in 2.5.x where a user could change cpu affinity (through proc or a syscall, though I don't think the patches providing this are accepted as yet).

Vamsi.

Quoted 18 lines · Daniel Jacobowitz
Quoted 7 lines · Vamsi Krishna S .

There is serialization at higher level. We take a write lock on current->mm->mmap_sem at the beginning of elf_core_dump function which is released just before leaving the function. So, if one thread enters elf_core_dump and starts dumping core, no other thread (same mm) of the same process can start dumping.
<snip>

That's not a feature, it's a bug. You can't take the mmap_sem before collecting thread status; it will cause a deadlock on at least ia64, where some registers are collected from user memory.

(Thanks to Manfred Spraul for explaining that to me.)

Signature · 2 lines
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer
Signature · 5 lines
Vamsi Krishna S.
Linux Technology Center,
IBM Software Lab, Bangalore.
Ph: +91 80 5262355 Extn: 3959
Internet: [email protected]
#8 lore raw
In reply to Vamsi Krishna S .
Quoted 5 lines · Vamsi Krishna S .

Dan,

Thanks for pointing this out. I see that this change has now gone into 2.4.18 as well as 2.5.4. We would ensure that the down_write happens only after the registers of all threads are collected.

Yes, your other patch for this looks OK.

Quoted 5 lines

Coming back to the original point raised by Pavel, indeed there is nothing preventing external code (any other kernel modules) modifying the cpus_allowed field from under us. This could get worse in 2.5.x where a user could change cpu affinity (through proc or a syscall, though I don't think the patches providing this are accepted as yet).

We really need a non-signal-based way to tell the scheduler that a task can not be scheduled. A lot of the machinery is all there, but private to sched.c; the rest is pretty straightforward.

Signature · 2 lines
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer
#9 lore raw
Quoted 3 lines

We really need a non-signal-based way to tell the scheduler that a task can not be scheduled. A lot of the machinery is all there, but private to sched.c; the rest is pretty straightforward.

You need interrupts to handle this, even if you don't wrap it in the top layer of signals it will be able to use much of the code I agree. The nasty case is the "currently running on another cpu" one. Especially since you can't just "trap it" - if you IPI that processor it might have moved by the time the IPI arrives 8)

#10 lore raw
Quoted 5 lines · Alan Cox

You need interrupts to handle this, even if you don't wrap it in the top layer of signals it will be able to use much of the code I agree. The nasty case is the "currently running on another cpu" one. Especially since you can't just "trap it" - if you IPI that processor it might have moved by the time the IPI arrives 8)

This why I grabbed all those locks, and did the two sets of IPI's in the tcore patch. Once the runqueue lock is grabbed, even if that process on the other CPU tries to migrate, it won't get swapped in or looked at by the
scheduler until its cpus_allowed member has been marked. After cpus_allowed
has been marked it won't run.

I don't think there is any faster way of getting the other CPU's into schedule and a specific running process to be swapped out than what was done here.

The only risk with this type of code is if other code or drivers attempt similar maneuvers at the same time. Having a standard mechanism or API for this in the scheduler would be a "good thing".

--mgross
ps.
I've just started considering how to do this with the 2.5 O(1) scheduler, and I'm not sure yet how I can accomplish this process "pausing" behavior just yet.

#11 lore raw
Quoted 2 lines

This why I grabbed all those locks, and did the two sets of IPI's in the tcore patch. Once the runqueue lock is grabbed, even if that process on the

If you IPI holding a lock whats going to happen if while the IPI is going across the cpus the other processor tries to grab the runqueue lock and is spinning on it with interrupts off ?

#12 lore raw
Quoted 7 lines · Alan Cox
Quoted 3 lines

 This why I grabbed all those locks, and did the two sets of IPI's in the tcore patch.  Once the runqueue lock is grabbed, even if that process on the

If you IPI holding a lock whats going to happen if while the IPI is going across the cpus the other processor tries to grab the runqueue lock and is spinning on it with interrupts off ?

Then the at least 2 CPU's would quickly become dead locked on the synchronization IPI this patch sends at the end of the suspend_other_threads function call.

Interrupts shouldn't be turned off when grabbing the runqueue lock. Its also a bad thing if they would happen to be off while calling into to schedule.

I think schedule was designed to be called only while interrupts are turned on. It BUG's if "in_interrupt" to enforce this.

--mgross

#13 lore raw
In reply to Alan Cox

Hi!

Quoted 11 lines
Quoted 5 lines

You need interrupts to handle this, even if you don't wrap it in the top layer of signals it will be able to use much of the code I agree. The nasty case is the "currently running on another cpu" one. Especially since you can't just "trap it" - if you IPI that processor it might have moved by the time the IPI arrives 8)

This why I grabbed all those locks, and did the two sets of IPI's in the tcore patch. Once the runqueue lock is grabbed, even if that process on the other CPU tries to migrate, it won't get swapped in or looked at by the
scheduler until its cpus_allowed member has been marked. After cpus_allowed
has been marked it won't run.

BTW it would be very nice to put "task freezing" in some generic place. I have my own version of task freezing (with refrigerator), and it would be good to be able to share that...

Quoted 3 lines

The only risk with this type of code is if other code or drivers attempt similar maneuvers at the same time. Having a standard mechanism or API for this in the scheduler would be a "good thing".

Ahha, so you know it, too.

Quoted 3 lines

I've just started considering how to do this with the 2.5 O(1) scheduler, and I'm not sure yet how I can accomplish this process "pausing" behavior just yet.

I'm doing this in my freezer, and it should be safe even on 2.5.X. Most interesting is the part in suspend.c...
Pavel

arch/i386/kernel/apm.c +1 -0
@@ -1664,6 +1664,7 @@
 	daemonize();
 
 	strcpy(current->comm, "kapmd");
+	current->flags |= PF_IOTHREAD;
 	sigfillset(&current->blocked);
 
 	if (apm_info.connection_version == 0) {
arch/i386/kernel/signal.c +7 -0
@@ -20,6 +20,7 @@
 #include <linux/stddef.h>
 #include <linux/tty.h>
 #include <linux/personality.h>
+#include <linux/suspend.h>
 #include <asm/ucontext.h>
 #include <asm/uaccess.h>
 #include <asm/i387.h>
@@ -595,6 +596,11 @@
 	if ((regs->xcs & 3) != 3)
 		return 1;
 
+	if (current->flags & PF_FREEZE) {
+		refrigerator(0);
+		goto no_signal;
+	}
+
 	if (!oldset)
 		oldset = &current->blocked;
 
@@ -705,6 +711,7 @@
 		return 1;
 	}
 
+ no_signal:
 	/* Did we come from a system call? */
 	if (regs->orig_eax >= 0) {
 		/* Restart the system call - no handlers present */
drivers/usb/storage/usb.c +1 -0
@@ -316,6 +316,7 @@
 	 */
 	exit_files(current);
 	current->files = init_task.files;
+	current->flags |= PF_IOTHREAD;
 	atomic_inc(&current->files->count);
 	daemonize();
 
fs/buffer.c +5 -1
@@ -129,6 +129,8 @@
 		wake_up(&bh->b_wait);
 }
 
+DECLARE_TASK_QUEUE(tq_bdflush);
+
 /*
  * Rewrote the wait-routines to use the "new" wait-queue functionality,
  * and getting rid of the cli-sti pairs. The wait-queue routines still
@@ -2981,12 +2986,14 @@
 	spin_unlock_irq(&tsk->sigmask_lock);
 
 	complete((struct completion *)startup);
-
+	current->flags |= PF_KERNTHREAD;
 	for (;;) {
 		wait_for_some_buffers(NODEV);
 
 		/* update interval */
 		interval = bdf_prm.b_un.interval;
+		if (current->flags & PF_FREEZE)
+			refrigerator(PF_IOTHREAD);
 		if (interval) {
 			tsk->state = TASK_INTERRUPTIBLE;
 			schedule_timeout(interval);
fs/jbd/journal.c +11 -1
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
+#include <linux/suspend.h>
 #include <asm/uaccess.h>
 #include <linux/proc_fs.h>
 
@@ -226,6 +227,7 @@
 			journal->j_commit_interval / HZ);
 	list_add(&journal->j_all_journals, &all_journals);
 
+	current->flags |= PF_KERNTHREAD;
 	/* And now, wait forever for commit wakeup events. */
 	while (1) {
 		if (journal->j_flags & JFS_UNMOUNT)
@@ -246,7 +248,15 @@
 		}
 
 		wake_up(&journal->j_wait_done_commit);
-		interruptible_sleep_on(&journal->j_wait_commit);
+		if (current->flags & PF_FREEZE) { /* The simpler the better. Flushing journal isn't a
+						     good idea, because that depends on threads that
+						     may be already stopped. */
+			jbd_debug(1, "Now suspending kjournald\n");
+			refrigerator(PF_IOTHREAD);
+			jbd_debug(1, "Resuming kjournald\n");						
+		} else		/* we assume on resume that commits are already there,
+				   so we don't sleep */
+			interruptible_sleep_on(&journal->j_wait_commit);
 
 		jbd_debug(1, "kjournald wakes\n");
 
include/linux/sched.h +4 -0
@@ -427,6 +427,10 @@
 #define PF_MEMDIE	0x00001000	/* Killed for out-of-memory */
 #define PF_FREE_PAGES	0x00002000	/* per process page freeing */
 #define PF_NOIO		0x00004000	/* avoid generating further I/O */
+#define PF_FROZEN	0x00008000	/* frozen for system suspend */
+#define PF_FREEZE	0x00010000	/* this task should be frozen for suspend */
+#define PF_IOTHREAD	0x00020000	/* this thread is needed for doing I/O to swap */
+#define PF_KERNTHREAD	0x00040000	/* this thread is a kernel thread that cannot be sent signals to */
 
 #define PF_USEDFPU	0x00100000	/* task used FPU this quantum (SMP) */
 
kernel/context.c +1 -0
@@ -72,6 +72,7 @@
 
 	daemonize();
 	strcpy(curtask->comm, "keventd");
+	current->flags |= PF_IOTHREAD;
 	keventd_running = 1;
 	keventd_task = curtask;
 
kernel/signal.c +1 -1
@@ -463,7 +463,7 @@
  * No need to set need_resched since signal event passing
  * goes through ->blocked
  */
-static inline void signal_wake_up(struct task_struct *t)
+inline void signal_wake_up(struct task_struct *t)
 {
 	t->sigpending = 1;
 
kernel/softirq.c +1 -0
@@ -366,6 +366,7 @@
 
 	daemonize();
 	current->nice = 19;
+	current->flags |= PF_IOTHREAD;
 	sigfillset(&current->blocked);
 
 	/* Migrate to the right CPU */
kernel/suspend.c +0 -0
@@ -0,0 +1,1373 @@

...
+/*
+ * Refrigerator and related stuff
+ */
+
+#define INTERESTING(p) \
+ /* We don't want to touch kernel_threads..*/ \
+ if (p->flags & PF_IOTHREAD) \
+ continue; \
+ if (p == current) \
+ continue; \
+ if (p->state == TASK_ZOMBIE) \
+ continue;
+
+/* Refrigerator is place where frozen processes are stored :-). */ +void refrigerator(unsigned long flag)
+{
+ /* You need correct to work with real-time processes.

+	   OTOH, this way one process may see (via /proc/) some other
+	   process in stopped state (and thereby discovered we were
+	   suspended. We probably do not care. 

+ */
+ long save;
+ save = current->state;
+ current->state = TASK_STOPPED;
+// PRINTK("%s entered refrigerator\n", current->comm);
+ printk(":");
+ current->flags &= ~PF_FREEZE;
+ if (flag)
+ flush_signals(current); /* We have signaled a kernel thread, which isn't normal behaviour
+ and that may lead to 100%CPU sucking because those threads
+ just don't manage signals. */
+ current->flags |= PF_FROZEN;
+ while (current->flags & PF_FROZEN)
+ schedule();
+// PRINTK("%s left refrigerator\n", current->comm);
+ printk(":");
+ current->state = save;
+}
+
+/* 0 = success, else # of processes that we failed to stop */ +static int freeze_processes(void)
+{
+ int todo, start_time;
+ struct task_struct *p;
+
+ PRINTS( "Waiting for tasks to stop... " );
+
+ start_time = jiffies;
+ do {
+ todo = 0;
+ read_lock(&tasklist_lock);
+ for_each_task(p) {
+ unsigned long flags;
+ INTERESTING(p);
+ if (p->flags & PF_FROZEN)
+ continue;
+
+ /* FIXME: smp problem here: we may not access other process' flags
+ without locking */
+ p->flags |= PF_FREEZE;
+ spin_lock_irqsave(&p->sigmask_lock, flags);
+ signal_wake_up(p);
+ spin_unlock_irqrestore(&p->sigmask_lock, flags);
+ todo++;
+ }
+ read_unlock(&tasklist_lock);
+ sys_sched_yield();
+ schedule();
+ if (time_after(jiffies, start_time + TIMEOUT)) {
+ PRINTK( "\n" );
+ printk(KERN_ERR " stopping tasks failed (%d tasks remaining)\n", todo );
+ return todo;
+ }
+ } while(todo);
+
+ PRINTK( " ok\n" );
+ return 0;
+}
+
+static void thaw_processes(void)
+{
+ struct task_struct *p;
+
+ PRINTR( "Restarting tasks..." );
+ read_lock(&tasklist_lock);
+ for_each_task(p) {
+ INTERESTING(p);
+
+ if (p->flags & PF_FROZEN) p->flags &= ~PF_FROZEN;
+ else
+ printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
+ wake_up_process(p);
+ }
+ read_unlock(&tasklist_lock);
+ PRINTK( " done\n" );
+ MDELAY(500);
+}

mm/vmscan.c +6 -2
@@ -723,18 +723,22 @@
 	 * us from recursively trying to free more memory as we're
 	 * trying to free the first piece of memory in the first place).
 	 */
-	tsk->flags |= PF_MEMALLOC;
+	tsk->flags |= PF_MEMALLOC | PF_KERNTHREAD;
 
 	/*
 	 * Kswapd main loop.
 	 */
 	for (;;) {
+		if (current->flags & PF_FREEZE)
+			refrigerator(PF_IOTHREAD);
 		__set_current_state(TASK_INTERRUPTIBLE);
 		add_wait_queue(&kswapd_wait, &wait);
 
 		mb();
-		if (kswapd_can_sleep())
+		if (kswapd_can_sleep()) {
 			schedule();
+		}
+		
 
 		__set_current_state(TASK_RUNNING);
 		remove_wait_queue(&kswapd_wait, &wait);
Signature · 2 lines
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.
#14 lore raw
In reply to Vamsi Krishna S .
Quoted 5 lines · Vamsi Krishna S .

Mark,

Does moving the down_write() to be after the registers of all threads are collected help? (This patch on top of our previous one)

Yes, moving the down_write to after the grabbing of the registers fixes the semi lock ups.

I need to move my Big Sur to RH7.2 to continue my validation. Its running the 7.1 libs and gdb / libpthreads.so aren't as happy at debug time as they are for 7.2 on ia32.

Thanks.

--mgross