* [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.