I think that the file kernel/fork.c should be patched as following:
--- linux-2.6.12-rc3.orig/kernel/fork.c 2005-05-03 22:31:21.000000000 -0300
+++ linux-2.6.12-rc3/kernel/fork.c 2005-05-04 09:37:46.000000000 -0300
@@ -429,7 +429,7 @@
tsk->vfork_done = NULL;
complete(vfork_done);
}
- if (tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
+ if (mm && tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
u32 __user * tidptr = tsk->clear_child_tid;
tsk->clear_child_tid = NULL;
If a process is killed and, for some reason, when closing all files a
pagefault is generated (should not happen, but...) after the function
exit_mm had been called, the kernel will try to exit this process again,
calling exit_mm and mm_release with a null mm, and will generate another
pagefault in atomic_read(&mm->mm_users), and so on and the system will
hang. I think this patch solve this problem.
On Wed, May 04, 2005 at 11:46:18AM -0300, Andr? Pereira de Almeida wrote:
> - if (tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
> + if (mm && tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
did you actually see a problem here?
Chris Wedgwood wrote:
>On Wed, May 04, 2005 at 11:46:18AM -0300, Andr? Pereira de Almeida wrote:
>
>
>
>>- if (tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
>>+ if (mm && tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
>>
>>
>
>did you actually see a problem here?
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
In a preemptible kernel with the serport module and a serial port try to
run the following program:
int main(int argc, char **argv)
{
int ldisc,fd;
fd = open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NONBLOCK);
ldisc = N_MOUSE;
ioctl(fd, TIOCSETD, &ldisc);
read(fd, NULL, 0);
return 0;
}
and kill it.
In my case it will hang the computer. I think this is a problem with the
serport module. With this patch, the serial mouse stop working, but the
computer don't hang.
On Wed, May 04, 2005 at 03:26:44PM -0300, Andr? Pereira de Almeida wrote:
> >>- if (tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
> >>+ if (mm && tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1)
> In a preemptible kernel with the serport module and a serial port try to
> run the following program:
>
> int main(int argc, char **argv)
> {
> int ldisc,fd;
>
> fd = open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NONBLOCK);
> ldisc = N_MOUSE;
> ioctl(fd, TIOCSETD, &ldisc);
> read(fd, NULL, 0);
> return 0;
> }
>
> and kill it. In my case it will hang the computer. I think this is
> a problem with the serport module. With this patch, the serial mouse
> stop working, but the computer don't hang.
then above something like:
BUG_ON(!mm);
or something might be better and eyeball the stack trace.
Chris Wedgwood wrote:
>On Wed, May 04, 2005 at 03:26:44PM -0300, Andr? Pereira de Almeida wrote:
>
>
>
>>>>- if (tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1) {
>>>>+ if (mm && tsk->clear_child_tid && atomic_read(&mm->mm_users) > 1)
>>>>
>>>>
>
>
>
>>In a preemptible kernel with the serport module and a serial port try to
>>run the following program:
>>
>>int main(int argc, char **argv)
>>{
>> int ldisc,fd;
>>
>> fd = open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NONBLOCK);
>> ldisc = N_MOUSE;
>> ioctl(fd, TIOCSETD, &ldisc);
>> read(fd, NULL, 0);
>> return 0;
>>}
>>
>>and kill it. In my case it will hang the computer. I think this is
>>a problem with the serport module. With this patch, the serial mouse
>>stop working, but the computer don't hang.
>>
>>
>
>then above something like:
>
> BUG_ON(!mm);
>
>or something might be better and eyeball the stack trace.
>
>
In my example, a stack trace will be already generated by an oops:
[4300748.423000] Unable to handle kernel paging request at virtual
address 6b6b6b7b
[4300748.423000] printing eip:
[4300748.423000] d0927366
[4300748.423000] *pde = 00000000
[4300748.423000] Oops: 0000 [#1]
[4300748.423000] PREEMPT
and so on, with a call trace:
[4300748.423000] Call Trace:
[4300748.423000] [<c0104bfa>] show_stack+0x7a/0x90
[4300748.423000] [<c0104d7d>] show_registers+0x14d/0x1b0
[4300748.423000] [<c0104fcc>] die+0x14c/0x2c0
[4300748.423000] [<c0118b6f>] do_page_fault+0x31f/0x638
[4300748.423000] [<c01046df>] error_code+0x4f/0x54
[4300748.423000] [<c02b88fd>] tty_wakeup+0x5d/0x60
I think that maybe it's good to put a:
WARN_ON(!mm);
but a BUG_ON or without this patch, the kernel will halt, even if the
problem is not so severe.
Andr?.
On Wed, 04 May 2005 15:26:44 -0300, =?ISO-8859-1?Q?Andr=E9_Pereira_de_Almeida?= said:
> In a preemptible kernel with the serport module and a serial port try to
> run the following program:
> and kill it.
> In my case it will hang the computer. I think this is a problem with the
> serport module. With this patch, the serial mouse stop working, but the
> computer don't hang.
The fact that the mouse stops working is indicative that this patch doesn't
actually fix the problem, it's just pushing it around in the kernel - sooner
or later something *else* is going to go pear-shaped on the null *mm. The right
fix is to figure out why mm is bogus and fix that issue.
> >>In a preemptible kernel with the serport module and a serial port try to
> >>run the following program:
> >>
> >>int main(int argc, char **argv)
> >>{
> >> int ldisc,fd;
> >>
> >> fd = open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NONBLOCK);
> >> ldisc = N_MOUSE;
> >> ioctl(fd, TIOCSETD, &ldisc);
> >> read(fd, NULL, 0);
> >> return 0;
> >>}
> >>
> >>and kill it. In my case it will hang the computer. I think this is
> >>a problem with the serport module. With this patch, the serial mouse
> >>stop working, but the computer don't hang.
> >>
> >>
> >
> >then above something like:
> >
> > BUG_ON(!mm);
> >
> >or something might be better and eyeball the stack trace.
> >
> >
> In my example, a stack trace will be already generated by an oops:
> [4300748.423000] Unable to handle kernel paging request at virtual
> address 6b6b6b7b
> [4300748.423000] printing eip:
> [4300748.423000] d0927366
> [4300748.423000] *pde = 00000000
> [4300748.423000] Oops: 0000 [#1]
> [4300748.423000] PREEMPT
> and so on, with a call trace:
> [4300748.423000] Call Trace:
> [4300748.423000] [<c0104bfa>] show_stack+0x7a/0x90
> [4300748.423000] [<c0104d7d>] show_registers+0x14d/0x1b0
> [4300748.423000] [<c0104fcc>] die+0x14c/0x2c0
> [4300748.423000] [<c0118b6f>] do_page_fault+0x31f/0x638
> [4300748.423000] [<c01046df>] error_code+0x4f/0x54
> [4300748.423000] [<c02b88fd>] tty_wakeup+0x5d/0x60
>
> I think that maybe it's good to put a:
> WARN_ON(!mm);
> but a BUG_ON or without this patch, the kernel will halt, even if the
> problem is not so severe.
Patching up the kernel hiding things that must not happen is not the way
to go. All kernel bugs are severe (as you just showed us!). Adding extra
checks like your original patch did may even cause much more harm
because it may hide other problems causing silent problems.
The kernel should scream as loudly as possible when something goes
wrong. Please get the underlying problem fixed instead.
On 5/4/05, [email protected] <[email protected]> wrote:
> On Wed, 04 May 2005 15:26:44 -0300, =?ISO-8859-1?Q?Andr=E9_Pereira_de_Almeida?= said:
>
> > In a preemptible kernel with the serport module and a serial port try to
> > run the following program:
>
> > and kill it.
> > In my case it will hang the computer. I think this is a problem with the
> > serport module. With this patch, the serial mouse stop working, but the
> > computer don't hang.
>
> The fact that the mouse stops working is indicative that this patch doesn't
> actually fix the problem, it's just pushing it around in the kernel - sooner
> or later something *else* is going to go pear-shaped on the null *mm. The right
> fix is to figure out why mm is bogus and fix that issue.
>
serport module is busted at the moment. Look here for the patch:
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.12-rc3/2.6.12-rc3-mm2/broken-out/serport-oops-fix.patch
It should probably go into -stable but Vojtech seems to have
disappeared... I think Fedora already picked it up.
--
Dmitry
[email protected] wrote:
>On Wed, 04 May 2005 15:26:44 -0300, =?ISO-8859-1?Q?Andr=E9_Pereira_de_Almeida?= said:
>
>
>
>>In a preemptible kernel with the serport module and a serial port try to
>>run the following program:
>>
>>
>
>
>
>>and kill it.
>>In my case it will hang the computer. I think this is a problem with the
>>serport module. With this patch, the serial mouse stop working, but the
>>computer don't hang.
>>
>>
>
>The fact that the mouse stops working is indicative that this patch doesn't
>actually fix the problem, it's just pushing it around in the kernel - sooner
>or later something *else* is going to go pear-shaped on the null *mm. The right
>fix is to figure out why mm is bogus and fix that issue.
>
>
This patch is not to fix this problem, but to minimize a bug in other
modules (in this case I think it's in the serport module) and to keep
the system working, at least to make a clean shutdown or terminate some
important process cleanly.
This patch is in the mm_release that should return a null *mm anyway.
The bogus mm came from a pagefault in the do_exit function in
kernel/exit.c that occured after the exit_mm call. This pagefault, in my
case, come from a filp_close call that called tty_wakeup. I'm trying to
identify this problem, but a pagefault like this may came from several
different modules.
Andr?
Alexander Nyberg <[email protected]> wrote:
>
> > [4300748.423000] Call Trace:
> > [4300748.423000] [<c0104bfa>] show_stack+0x7a/0x90
> > [4300748.423000] [<c0104d7d>] show_registers+0x14d/0x1b0
> > [4300748.423000] [<c0104fcc>] die+0x14c/0x2c0
> > [4300748.423000] [<c0118b6f>] do_page_fault+0x31f/0x638
> > [4300748.423000] [<c01046df>] error_code+0x4f/0x54
> > [4300748.423000] [<c02b88fd>] tty_wakeup+0x5d/0x60
> >
> > I think that maybe it's good to put a:
> > WARN_ON(!mm);
> > but a BUG_ON or without this patch, the kernel will halt, even if the
> > problem is not so severe.
>
> Patching up the kernel hiding things that must not happen is not the way
> to go. All kernel bugs are severe (as you just showed us!). Adding extra
> checks like your original patch did may even cause much more harm
> because it may hide other problems causing silent problems.
If I understand Andre correctly, his patch will prevent infinite recursion
in the oops path - if some process oopses after having run exit_mm().
If so then it's a reasonable debugging aid. Although there might be better
places to do it, such as
if (!current->i_tried_to_exit++)
return;
in do_exit(). Dunno.
Andrew Morton wrote:
>Alexander Nyberg <[email protected]> wrote:
>
>
>>>[4300748.423000] Call Trace:
>>>[4300748.423000] [<c0104bfa>] show_stack+0x7a/0x90
>>>[4300748.423000] [<c0104d7d>] show_registers+0x14d/0x1b0
>>>[4300748.423000] [<c0104fcc>] die+0x14c/0x2c0
>>>[4300748.423000] [<c0118b6f>] do_page_fault+0x31f/0x638
>>>[4300748.423000] [<c01046df>] error_code+0x4f/0x54
>>>[4300748.423000] [<c02b88fd>] tty_wakeup+0x5d/0x60
>>>
>>>I think that maybe it's good to put a:
>>> WARN_ON(!mm);
>>>but a BUG_ON or without this patch, the kernel will halt, even if the
>>>problem is not so severe.
>>>
>>>
>>Patching up the kernel hiding things that must not happen is not the way
>>to go. All kernel bugs are severe (as you just showed us!). Adding extra
>>checks like your original patch did may even cause much more harm
>>because it may hide other problems causing silent problems.
>>
>>
>
>If I understand Andre correctly, his patch will prevent infinite recursion
>in the oops path - if some process oopses after having run exit_mm().
>
>If so then it's a reasonable debugging aid. Although there might be better
>places to do it, such as
>
> if (!current->i_tried_to_exit++)
> return;
>
>in do_exit(). Dunno.
>
>
>
I agree. If you need this patch, the you are in already in trouble, your
kernel had already oopsed, your system is halted and you can't even send
a bug report because you can't read read the call trace (it's an
infinite recursion) and you can see only the end of it. It's not
intended to hide the problem, the problem will be already in your screen.
Andr?.
> > > I think that maybe it's good to put a:
> > > WARN_ON(!mm);
> > > but a BUG_ON or without this patch, the kernel will halt, even if the
> > > problem is not so severe.
> >
> > Patching up the kernel hiding things that must not happen is not the way
> > to go. All kernel bugs are severe (as you just showed us!). Adding extra
> > checks like your original patch did may even cause much more harm
> > because it may hide other problems causing silent problems.
>
> If I understand Andre correctly, his patch will prevent infinite recursion
> in the oops path - if some process oopses after having run exit_mm().
>
> If so then it's a reasonable debugging aid. Although there might be better
> places to do it, such as
>
> if (!current->i_tried_to_exit++)
> return;
>
> in do_exit(). Dunno.
This patch is very crude but it is quite resistant to recursive faults
in do_exit(), survives the LTP hammering I've given it. The problem is
not knowing where in the previous path it broke down so I'd rather just
leave it lying around and try a graceful reset/power off. But if anyone
has a better suggestion than the msleep() I'm all ears but this area is
sensitive.
Where is that anonymous patch hot-line...
Index: mm/kernel/exit.c
===================================================================
--- mm.orig/kernel/exit.c 2005-05-04 22:24:57.000000000 +0200
+++ mm/kernel/exit.c 2005-05-04 23:19:08.000000000 +0200
@@ -29,6 +29,7 @@
#include <linux/perfctr.h>
#include <linux/syscalls.h>
#include <linux/signal.h>
+#include <linux/delay.h>
#include <asm/uaccess.h>
#include <asm/unistd.h>
@@ -797,6 +798,14 @@
ptrace_notify((PTRACE_EVENT_EXIT << 8) | SIGTRAP);
}
+ /* We're taking recursive faults originating here in do_exit. Safest
+ * is to just leave this task alone and wait for reboot. */
+ if (tsk->flags & PF_EXITING) {
+ printk(KERN_ALERT "\nFixing recursive fault but reboot is needed!\n");
+ for (;;)
+ msleep(1000 * 10);
+ }
+
tsk->flags |= PF_EXITING;
/*
On Wed, 2005-05-04 at 23:21 +0200, Alexander Nyberg wrote:
> This patch is very crude but it is quite resistant to recursive faults
> in do_exit(), survives the LTP hammering I've given it. The problem is
> not knowing where in the previous path it broke down so I'd rather just
> leave it lying around and try a graceful reset/power off. But if anyone
> has a better suggestion than the msleep() I'm all ears but this area is
> sensitive.
>
> Where is that anonymous patch hot-line...
>
> + /* We're taking recursive faults originating here in do_exit. Safest
> + * is to just leave this task alone and wait for reboot. */
> + if (tsk->flags & PF_EXITING) {
> + printk(KERN_ALERT "\nFixing recursive fault but reboot is needed!\n");
> + for (;;)
> + msleep(1000 * 10);
> + }
> +
> tsk->flags |= PF_EXITING;
>
Instead of the for(;;) msleep, why not just take it permanently off the
run queue? With the following:
set_current_state(TASK_UNINTERRUPTIBLE);
schedule();
It basically gives the same effect, but is cleaner.
-- Steve
> Instead of the for(;;) msleep, why not just take it permanently off the
> run queue? With the following:
>
> set_current_state(TASK_UNINTERRUPTIBLE);
> schedule();
>
> It basically gives the same effect, but is cleaner.
Ah perfect, now it looks ok. Thanks!
Prevent recursive faults in do_exit() by leaving the task alone and wait
for reboot. This may allow a more graceful shutdown and possibly save
the original oops.
Signed-off-by: Alexander Nyberg <[email protected]>
Index: mm/kernel/exit.c
===================================================================
--- mm.orig/kernel/exit.c 2005-05-05 16:44:20.000000000 +0200
+++ mm/kernel/exit.c 2005-05-05 17:29:40.000000000 +0200
@@ -797,6 +797,14 @@
ptrace_notify((PTRACE_EVENT_EXIT << 8) | SIGTRAP);
}
+ /* We're taking recursive faults here in do_exit. Safest
+ * is to just leave this task alone and wait for reboot. */
+ if (unlikely(tsk->flags & PF_EXITING)) {
+ printk(KERN_ALERT "\nFixing recursive fault but reboot is needed!\n");
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule();
+ }
+
tsk->flags |= PF_EXITING;
/*
Alexander Nyberg <[email protected]> wrote:
>
> + /* We're taking recursive faults here in do_exit. Safest
> + * is to just leave this task alone and wait for reboot. */
I find this comment-block style a bit hard to maintain, and am anal about
consistency.
> + if (unlikely(tsk->flags & PF_EXITING)) {
> + printk(KERN_ALERT "\nFixing recursive fault but reboot is needed!\n");
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + schedule();
> + }
> +
In the printk string, a \n will terminate the current facility level, so
your KERN_ALERT there is a no-op. I simply removed it, which might cause
messy output sometimes but that seems better than always adding a newline.
--- 25/kernel/exit.c~avoid-recursive-oopses 2005-05-06 18:03:45.000000000 -0700
+++ 25-akpm/kernel/exit.c 2005-05-06 18:06:01.000000000 -0700
@@ -795,6 +795,17 @@ fastcall NORET_TYPE void do_exit(long co
ptrace_notify((PTRACE_EVENT_EXIT << 8) | SIGTRAP);
}
+ /*
+ * We're taking recursive faults here in do_exit. Safest is to just
+ * leave this task alone and wait for reboot.
+ */
+ if (unlikely(tsk->flags & PF_EXITING)) {
+ printk(KERN_ALERT
+ "Fixing recursive fault but reboot is needed!\n");
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule();
+ }
+
tsk->flags |= PF_EXITING;
/*
_