* [patch] oom-killer sysrq-f fix
@ 2005-03-25 0:33 Coywolf Qi Hunt
2005-03-25 0:44 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Coywolf Qi Hunt @ 2005-03-25 0:33 UTC (permalink / raw)
To: linux-kernel; +Cc: james4765, akpm
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 <coywolf@lovecn.org>
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] oom-killer sysrq-f fix
2005-03-25 0:33 [patch] oom-killer sysrq-f fix Coywolf Qi Hunt
@ 2005-03-25 0:44 ` Andrew Morton
2005-03-25 1:04 ` Coywolf Qi Hunt
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2005-03-25 0:44 UTC (permalink / raw)
To: coywolf; +Cc: coywolf, linux-kernel, james4765
Coywolf Qi Hunt <coywolf@sosdg.org> 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 <coywolf@lovecn.org>
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] oom-killer sysrq-f fix
2005-03-25 0:44 ` Andrew Morton
@ 2005-03-25 1:04 ` Coywolf Qi Hunt
2005-03-25 1:21 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Coywolf Qi Hunt @ 2005-03-25 1:04 UTC (permalink / raw)
To: Andrew Morton; +Cc: coywolf, coywolf, linux-kernel, james4765
Andrew Morton wrote:
> Coywolf Qi Hunt <coywolf@sosdg.org> 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 <coywolf@lovecn.org>
>>
>>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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] oom-killer sysrq-f fix
2005-03-25 1:04 ` Coywolf Qi Hunt
@ 2005-03-25 1:21 ` Andrew Morton
2005-03-25 7:36 ` Coywolf Qi Hunt
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2005-03-25 1:21 UTC (permalink / raw)
To: Coywolf Qi Hunt; +Cc: coywolf, coywolf, linux-kernel, james4765
Coywolf Qi Hunt <coywolf@lovecn.org> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] oom-killer sysrq-f fix
2005-03-25 1:21 ` Andrew Morton
@ 2005-03-25 7:36 ` Coywolf Qi Hunt
2005-03-25 9:59 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Coywolf Qi Hunt @ 2005-03-25 7:36 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, james4765
On Thu, Mar 24, 2005 at 05:21:27PM -0800, Andrew Morton wrote:
> Coywolf Qi Hunt <coywolf@lovecn.org> 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 <coywolf@lovecn.org>
---
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] oom-killer sysrq-f fix
2005-03-25 7:36 ` Coywolf Qi Hunt
@ 2005-03-25 9:59 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2005-03-25 9:59 UTC (permalink / raw)
To: coywolf; +Cc: coywolf, linux-kernel, james4765
Coywolf Qi Hunt <coywolf@sosdg.org> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-03-25 10:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-25 0:33 [patch] oom-killer sysrq-f fix Coywolf Qi Hunt
2005-03-25 0:44 ` Andrew Morton
2005-03-25 1:04 ` Coywolf Qi Hunt
2005-03-25 1:21 ` Andrew Morton
2005-03-25 7:36 ` Coywolf Qi Hunt
2005-03-25 9:59 ` Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.