Messages in this thread | | | Date | Sat, 31 Oct 2009 16:58:48 +0900 | From | Naohiro Ooiwa <> | Subject | Re: [PATCH] show message when exceeded rlimit of pending signals |
| |
Andrew Morton wrote: > On Fri, 30 Oct 2009 20:36:31 +0900 > Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote: > >> Hi Ingo, >> >> I wrote proper changelog entry. >> And I resent the patch. I added KERN_INFO to printk. >> >> >> >> When the system has too many timers or too many aggregate >> queued signals, the EAGAIN error is returned to application >> from kernel, including timer_create(). >> It means that exceeded limit of pending signals at all. >> But we can't imagine it. >> >> This patch show the message when reached limit of pending signals. >> If you see this message and your system behaved unexpectedly, >> you can run following command. >> # ulimit -i unlimited >> >> With help from Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>. >> >> >> ... >> >> diff --git a/kernel/signal.c b/kernel/signal.c >> index 6705320..50e10dc 100644 >> --- a/kernel/signal.c >> +++ b/kernel/signal.c >> @@ -41,6 +41,8 @@ >> >> static struct kmem_cache *sigqueue_cachep; >> >> +int print_fatal_signals __read_mostly; >> + >> static void __user *sig_handler(struct task_struct *t, int sig) >> { >> return t->sighand->action[sig - 1].sa.sa_handler; >> @@ -188,6 +190,14 @@ int next_signal(struct sigpending *pending, sigset_t *mask) >> return sig; >> } >> >> +static void show_reach_rlimit_sigpending(void) >> +{ >> + if (!printk_ratelimit()) >> + return; > > printk_ratelimit() is a bad thing and we should be working toward > removing it altogether, not adding new callers. > > Because it uses global state. So if subsystem A is trying to generate > lots of printk's, subsystem B's important message might get > accidentally suppressed. > > It's better to use DEFINE_RATELIMIT_STATE() and __ratelimit() directly.
Thank you for your advices. And I was glad to talk to you in Japan Linux Symposium.
I got it, now that you mention it. I will fix my patch.
> >> + printk(KERN_INFO "%s/%d: reached the limit of pending signals.\n", >> + current->comm, current->pid); > > I suggest that this be > > "reached RLIMIT_SIGPENDING" > > because RLIMIT_SIGPENDING is a well-understood term and concept. >
OK, I see.
>> static void print_fatal_signal(struct pt_regs *regs, int signr) >> { >> - printk("%s/%d: potentially unexpected fatal signal %d.\n", >> + printk(KERN_INFO "%s/%d: potentially unexpected fatal signal %d.\n", >> current->comm, task_pid_nr(current), signr); >> > > This is an unchangelogged, unrelated, non-backward-compatible > user-visible change. For some people, their machine which used to > print this warning will mysteriously stop doing so when they upgrade > their kernels. > > That doesn't mean that we shouldn't make the change. But we should > have a think about it and we shouldn't hide changes of this nature > inside some other patch like this. >
You are right. I'm sorry, I shouldn't habe done it.
Thanks you. Naohiro Ooiwa
| |