lkml.org 
[lkml]   [2009]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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




\
 
 \ /
  Last update: 2009-10-24 09:05    [W:0.058 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site