Hello,
akpm, I saw you noticed it,
http://www.ussg.iu.edu/hypermail/linux/kernel/0412.0/0424.html
Jim Nelson, this patch is to your post: 2.6.11-rc2-mm2 - kernel panic with SysRq-f
Recent make-sysrq-f-call-oom_kill.patch calls oom-killer in interrupt context,
thus results into panic. This patch fixes out_of_memory() to avoid schedule
when in interrupt context.
Coywolf
Signed-off-by: Coywolf Qi Hunt <[email protected]>
diff -pruN 2.6.12-rc1-mm2/mm/oom_kill.c 2.6.12-rc1-mm2-cy/mm/oom_kill.c
--- 2.6.12-rc1-mm2/mm/oom_kill.c 2005-03-03 17:12:18.000000000 +0800
+++ 2.6.12-rc1-mm2-cy/mm/oom_kill.c 2005-03-25 08:07:19.000000000 +0800
@@ -20,6 +20,7 @@
#include <linux/swap.h>
#include <linux/timex.h>
#include <linux/jiffies.h>
+#include <linux/hardirq.h>
/* #define DEBUG */
@@ -283,6 +284,9 @@ retry:
if (mm)
mmput(mm);
+ if (in_interrupt())
+ return;
+
/*
* Give "p" a good chance of killing itself before we
* retry to allocate memory.
Coywolf Qi Hunt <[email protected]> wrote:
>
> Recent make-sysrq-f-call-oom_kill.patch calls oom-killer in interrupt context,
> thus results into panic. This patch fixes out_of_memory() to avoid schedule
> when in interrupt context.
>
> Coywolf
>
>
> Signed-off-by: Coywolf Qi Hunt <[email protected]>
>
> diff -pruN 2.6.12-rc1-mm2/mm/oom_kill.c 2.6.12-rc1-mm2-cy/mm/oom_kill.c
> --- 2.6.12-rc1-mm2/mm/oom_kill.c 2005-03-03 17:12:18.000000000 +0800
> +++ 2.6.12-rc1-mm2-cy/mm/oom_kill.c 2005-03-25 08:07:19.000000000 +0800
> @@ -20,6 +20,7 @@
> #include <linux/swap.h>
> #include <linux/timex.h>
> #include <linux/jiffies.h>
> +#include <linux/hardirq.h>
>
> /* #define DEBUG */
>
> @@ -283,6 +284,9 @@ retry:
> if (mm)
> mmput(mm);
>
> + if (in_interrupt())
> + return;
That'll make the whole feature a no-op, won't it?
The thing needs to be moved into process context via schedule_work(). I
haven't got around to it yet.
Andrew Morton wrote:
> Coywolf Qi Hunt <[email protected]> wrote:
>
>>Recent make-sysrq-f-call-oom_kill.patch calls oom-killer in interrupt context,
>>thus results into panic. This patch fixes out_of_memory() to avoid schedule
>>when in interrupt context.
>>
>> Coywolf
>>
>>
>>Signed-off-by: Coywolf Qi Hunt <[email protected]>
>>
>>diff -pruN 2.6.12-rc1-mm2/mm/oom_kill.c 2.6.12-rc1-mm2-cy/mm/oom_kill.c
>>--- 2.6.12-rc1-mm2/mm/oom_kill.c 2005-03-03 17:12:18.000000000 +0800
>>+++ 2.6.12-rc1-mm2-cy/mm/oom_kill.c 2005-03-25 08:07:19.000000000 +0800
>>@@ -20,6 +20,7 @@
>> #include <linux/swap.h>
>> #include <linux/timex.h>
>> #include <linux/jiffies.h>
>>+#include <linux/hardirq.h>
>>
>> /* #define DEBUG */
>>
>>@@ -283,6 +284,9 @@ retry:
>> if (mm)
>> mmput(mm);
>>
>>+ if (in_interrupt())
>>+ return;
>
>
> That'll make the whole feature a no-op, won't it?
It won't be a no-op. I have tested it. It works well.
I pressed sysrq-f, loging bash got killed and I had to re-login.
>
> The thing needs to be moved into process context via schedule_work(). I
> haven't got around to it yet.
>
I don't think schedule_work() is a good option here. Since sysrq-f is emergent,
we just let oom-killer send SIGKILL in interrupt context and return.
We needn't send SIGKILL in a process context. That'll be slow and [events] may got delayed.
Coywolf Qi Hunt <[email protected]> wrote:
>
> >>--- 2.6.12-rc1-mm2/mm/oom_kill.c 2005-03-03 17:12:18.000000000 +0800
> >>+++ 2.6.12-rc1-mm2-cy/mm/oom_kill.c 2005-03-25 08:07:19.000000000 +0800
> >>@@ -20,6 +20,7 @@
> >> #include <linux/swap.h>
> >> #include <linux/timex.h>
> >> #include <linux/jiffies.h>
> >>+#include <linux/hardirq.h>
> >>
> >> /* #define DEBUG */
> >>
> >>@@ -283,6 +284,9 @@ retry:
> >> if (mm)
> >> mmput(mm);
> >>
> >>+ if (in_interrupt())
> >>+ return;
> >
> >
> > That'll make the whole feature a no-op, won't it?
>
> It won't be a no-op. I have tested it. It works well.
> I pressed sysrq-f, loging bash got killed and I had to re-login.
(looks)
OK. But the patch is still deadlocky because we do task_lock() from
interrupt context.
> >
> > The thing needs to be moved into process context via schedule_work(). I
> > haven't got around to it yet.
> >
>
> I don't think schedule_work() is a good option here. Since sysrq-f is emergent,
> we just let oom-killer send SIGKILL in interrupt context and return.
>
> We needn't send SIGKILL in a process context. That'll be slow and [events] may got delayed.
There isn't much choice. It should work OK - schedule_task doesn't
allocate any memory.
keventd could be off allocating some memory somewhere, in which case it
could take some time to respond, but it isn't worth running another kernel
thread for sysrq-f.
On Thu, Mar 24, 2005 at 05:21:27PM -0800, Andrew Morton wrote:
> Coywolf Qi Hunt <[email protected]> wrote:
> >
> > >>--- 2.6.12-rc1-mm2/mm/oom_kill.c 2005-03-03 17:12:18.000000000 +0800
> > >>+++ 2.6.12-rc1-mm2-cy/mm/oom_kill.c 2005-03-25 08:07:19.000000000 +0800
> > >>@@ -20,6 +20,7 @@
> > >> #include <linux/swap.h>
> > >> #include <linux/timex.h>
> > >> #include <linux/jiffies.h>
> > >>+#include <linux/hardirq.h>
> > >>
> > >> /* #define DEBUG */
> > >>
> > >>@@ -283,6 +284,9 @@ retry:
> > >> if (mm)
> > >> mmput(mm);
> > >>
> > >>+ if (in_interrupt())
> > >>+ return;
> > >
> > >
> > > That'll make the whole feature a no-op, won't it?
> >
> > It won't be a no-op. I have tested it. It works well.
> > I pressed sysrq-f, loging bash got killed and I had to re-login.
s/loging bash/login bash/
>
> (looks)
>
> OK. But the patch is still deadlocky because we do task_lock() from
> interrupt context.
Yes, it's deadlocky, but hardly observable in practice. SysRq is just
"taking the risk yourself" to users.
OK, the following patch put moom into workqueue.
Do you agree to put sysrq-e and sysrq-i into workqueue too?
send_sig_all() should do task_lock() too.
Signed-off-by: Coywolf Qi Hunt <[email protected]>
---
diff -pruN 2.6.12-rc1-mm2/drivers/char/sysrq.c 2.6.12-rc1-mm2-cy/drivers/char/sysrq.c
--- 2.6.12-rc1-mm2/drivers/char/sysrq.c 2005-03-25 05:21:39.000000000 +0800
+++ 2.6.12-rc1-mm2-cy/drivers/char/sysrq.c 2005-03-25 13:28:33.000000000 +0800
@@ -34,6 +34,7 @@
#include <linux/swap.h>
#include <linux/spinlock.h>
#include <linux/vt_kern.h>
+#include <linux/workqueue.h>
#include <asm/ptrace.h>
#ifdef CONFIG_KGDB_SYSRQ
@@ -228,12 +229,19 @@ static struct sysrq_key_op sysrq_term_op
.enable_mask = SYSRQ_ENABLE_SIGNAL,
};
-static void sysrq_handle_moom(int key, struct pt_regs *pt_regs,
- struct tty_struct *tty)
+static void moom_callback(void *ignored)
{
out_of_memory(GFP_KERNEL);
// console_loglevel = 8;
}
+
+static DECLARE_WORK(moom_work, moom_callback, NULL);
+
+static void sysrq_handle_moom(int key, struct pt_regs *pt_regs,
+ struct tty_struct *tty)
+{
+ schedule_work(&moom_work);
+}
static struct sysrq_key_op sysrq_moom_op = {
.handler = sysrq_handle_moom,
.help_msg = "Full",
>
> > >
> > > The thing needs to be moved into process context via schedule_work(). I
> > > haven't got around to it yet.
> > >
> >
> > I don't think schedule_work() is a good option here. Since sysrq-f is emergent,
> > we just let oom-killer send SIGKILL in interrupt context and return.
> >
> > We needn't send SIGKILL in a process context. That'll be slow and [events] may got delayed.
>
> There isn't much choice. It should work OK - schedule_task doesn't
> allocate any memory.
>
> keventd could be off allocating some memory somewhere, in which case it
> could take some time to respond, but it isn't worth running another kernel
> thread for sysrq-f.
Coywolf Qi Hunt <[email protected]> wrote:
>
> OK, the following patch put moom into workqueue.
hm. Simple, isn't it?
> Do you agree to put sysrq-e and sysrq-i into workqueue too?
If you spy a deadlock then yes, I suppose we should.