Re: [PATCH] show message when exceeded rlimit of pending signals

[Naohiro Ooiwa <nooiwa@miraclelinux.com>]

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