Messages in this thread | | | Date | Sat, 24 Oct 2009 16:02:57 +0900 | From | Naohiro Ooiwa <> | Subject | Re: [PATCH] show message when exceeded rlimit of pending signals |
| |
Hi Ingo
Thank you so much for early quick reply. and I'm happy you agree with my proposal.
> Regarding the patch, i've got a few (very) small suggestions.
Thank you for pointing out. Please wait a moment. I will resend a patch.
Of course, I will plan to use print_ratelimit(). Actually, I received with same opinion from OGAWA-san.
Thank you Naohiro Ooiwa.
Ingo Molnar wrote: > * Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote: > >> Hi Andrew, >> >> I was glad to talk to you in Japan Linux Symposium. >> I'm writing about it. >> >> >> I'm working to support kernel. >> Recently, I got a inquiry about unexpected system behavior. >> I analyzed application of our customer includeing kernel. >> >> Eventually, there was no bug in application or kernel. >> I found the cause was the limit of pending signals. >> I ran following command. and system behaved expectedly. >> # ulimit -i unlimited >> >> When system behaved unexpectedly, the timer_create() in application >> had returned -EAGAIN value. >> But we can't imagine the -EAGAIN means that it exceeded limit of >> pending signals at all. >> >> Then I thought kernel should at least show some message about it. >> And I tried to create a patch. >> >> I'm sure that system engineeres will not have to have the same experience as I did. >> How do you think about this idea ? >> >> Thank you >> Naohiro Ooiwa. >> >> Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com> >> --- >> kernel/signal.c | 13 +++++++++++++ >> 1 files changed, 13 insertions(+), 0 deletions(-) >> >> diff --git a/kernel/signal.c b/kernel/signal.c >> index 6705320..0bc4934 100644 >> --- a/kernel/signal.c >> +++ b/kernel/signal.c >> @@ -188,6 +188,9 @@ int next_signal(struct sigpending *pending, sigset_t *mask) >> return sig; >> } >> >> +#define MAX_RLIMIT_CAUTION 5 >> +static int rlimit_caution_count = 0; >> + >> /* >> * allocate a new signal queue record >> * - this may be called without locks if and only if t == current, otherwise an >> @@ -211,6 +214,16 @@ static struct sigqueue *__sigqueue_alloc(struct task_struct *t, gfp_t flags, >> atomic_read(&user->sigpending) <= >> t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur) >> q = kmem_cache_alloc(sigqueue_cachep, flags); >> + else { >> + if (rlimit_caution_count <= MAX_RLIMIT_CAUTION ){ >> + printk(KERN_WARNING "reached the limit of pending signalis on pid %d\n", current->pid); >> + /* Last time, show the advice */ >> + if (rlimit_caution_count == MAX_RLIMIT_CAUTION) >> + printk(KERN_WARNING "If unexpected your system behavior, you can try ulimit -i unlimited\n"); >> + rlimit_caution_count++; >> + } >> + } >> + >> if (unlikely(q == NULL)) { >> atomic_dec(&user->sigpending); >> free_uid(user); > > This new warning looks quite useful, i've seen several apps get into > trouble silently due to that, again and again. > > The memory overhead of the signal queue was a problem 15 years ago ... > not so much today and people (and apps) dont expect to get in trouble > here. So the limit and its defaults are somewhat arcane, and the > behavior is catastrophic and hard to debug (because it's a dynamic > failure). > > Regarding the patch, i've got a few (very) small suggestions. > > Firstly, please update the if / else sequence from: > > if (...) > ... > else { > ... > } > > to: > > if (...) { > ... > } else { > ... > } > > as we strive for curly brace symmetries. > > also, a small typo: s/signalis/signals > > Plus, instead of using a pre-cooked global limit print_ratelimit() could > be used as well. That makes it useful for long-lived systems that run > into this limit occasionally. We wont spam the log - nor will we lose > (potentially essential) messages in the process. > > Thanks, > > Ingo
| |