[PATCH] show message when exceeded rlimit of pending signals

[PATCH] show message when exceeded rlimit of pending signals

[Naohiro Ooiwa]

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

Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
---
 Documentation/kernel-parameters.txt |   11 +++++++++--
 kernel/signal.c                     |   20 ++++++++++++++++----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9107b38..3bbd92f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2032,8 +2032,15 @@ and is between 256 and 4096 characters. It is defined in the file

 	print-fatal-signals=
 			[KNL] debug: print fatal signals
-			print-fatal-signals=1: print segfault info to
-			the kernel console.
+
+			If enabled, warn about various signal handling
+			related application anomalies: too many signals,
+			too many POSIX.1 timers, fatal signals causing a
+			coredump - etc.
+
+			If you hit the warning due to signal overflow,
+			you might want to try "ulimit -i unlimited".
+
 			default: off.

 	printk.time=	Show timing data prefixed to each printk message line
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(KERN_INFO "%s/%d: reached the limit of pending signals.\n",
+				current->comm, current->pid);
+}
+
 /*
  * allocate a new signal queue record
  * - this may be called without locks if and only if t == current, otherwise an
@@ -209,8 +219,12 @@ static struct sigqueue *__sigqueue_alloc(struct task_struct *t, gfp_t flags,
 	atomic_inc(&user->sigpending);
 	if (override_rlimit ||
 	    atomic_read(&user->sigpending) <=
-			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
+			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur) {
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
+	} else {
+		if (print_fatal_signals)
+			show_reach_rlimit_sigpending();
+	}
 	if (unlikely(q == NULL)) {
 		atomic_dec(&user->sigpending);
 		free_uid(user);
@@ -925,11 +939,9 @@ static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
 	return __send_signal(sig, info, t, group, from_ancestor_ns);
 }

-int print_fatal_signals;
-
 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);

 #if defined(__i386__) && !defined(__arch_um__)

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

[Andrew Morton]

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.


> +	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.

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

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

[Joe Perches]

On Fri, 2009-10-30 at 14:33 -0700, Andrew Morton wrote:
> On Fri, 30 Oct 2009 20:36:31 +0900
> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> > +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.

http://lkml.org/lkml/2009/9/21/323

I think there should be a generic kernel.h macro for this.
Something like:

#define printk_ratelimited(fmt, arg...)			\
({	static struct ratelimit_state _rs = {		\
		.interval = DEFAULT_RATELIMIT_INTERVAL,	\
		.burst = DEFAULT_RATELIMIT_BURST,	\
	};						\
	int rtn;					\
							\
	if (!__ratelimit(&_rs))				\
		rtn = printk(fmt, ##arg);		\
	else						\
		rtn = 0;				\
	rtn;						\
})
#define pr_info_rl(fmt, arg) \
	printk_ratelimited(KERN_INFO pr_fmt(fmt), ##arg)
etc...

[PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Joe Perches]

Add a printk_ratelimited statement expression macro
that uses a per-call ratelimit_state so that multiple
subsystems output messages are not suppressed by a global
__ratelimit state.

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f4e3184..555560c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -407,6 +407,50 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 #endif
 
 /*
+ * ratelimited messages with local ratelimit_state,
+ * no local ratelimit_state used in the !PRINTK case
+ */
+#ifdef CONFIG_PRINTK
+#define printk_ratelimited(fmt, ...)  ({		\
+	static struct ratelimit_state _rs = {		\
+		.interval = DEFAULT_RATELIMIT_INTERVAL, \
+		.burst = DEFAULT_RATELIMIT_BURST,       \
+	};                                              \
+							\
+	if (!__ratelimit(&_rs))                         \
+		printk(fmt, ##__VA_ARGS__);		\
+})
+#else
+/* No effect, but we still get type checking even in the !PRINTK case: */
+#define printk_ratelimited printk
+#endif
+
+#define pr_emerg_rl(fmt, ...) \
+        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_alert_rl(fmt, ...) \
+        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_crit_rl(fmt, ...) \
+        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_err_rl(fmt, ...) \
+        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_warning_rl(fmt, ...) \
+        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_notice_rl(fmt, ...) \
+        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_info_rl(fmt, ...) \
+        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+/* no pr_cont_rl, don't do that... */
+/* If you are writing a driver, please use dev_dbg instead */
+#if defined(DEBUG)
+#define pr_debug_rl(fmt, ...) \
+	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#else
+#define pr_debug_rl(fmt, ...) \
+	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
+				     ##__VA_ARGS__); 0; })
+#endif
+
+/*
  * General tracing related utility functions - trace_printk(),
  * tracing_on/tracing_off and tracing_start()/tracing_stop
  *

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

[Naohiro Ooiwa]

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

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

[Naohiro Ooiwa]

Naohiro Ooiwa wrote:
> Andrew Morton wrote:
>> On Fri, 30 Oct 2009 20:36:31 +0900
>> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>>>
>>> +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.

I fixed my patch.
Could you please check it.

Thanks you.
Naohiro Ooiwa


Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
---
 Documentation/kernel-parameters.txt |   11 +++++++++--
 kernel/signal.c                     |   21 ++++++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 9107b38..3bbd92f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2032,8 +2032,15 @@ and is between 256 and 4096 characters. It is defined in
the file

 	print-fatal-signals=
 			[KNL] debug: print fatal signals
-			print-fatal-signals=1: print segfault info to
-			the kernel console.
+
+			If enabled, warn about various signal handling
+			related application anomalies: too many signals,
+			too many POSIX.1 timers, fatal signals causing a
+			coredump - etc.
+
+			If you hit the warning due to signal overflow,
+			you might want to try "ulimit -i unlimited".
+
 			default: off.

 	printk.time=	Show timing data prefixed to each printk message line
diff --git a/kernel/signal.c b/kernel/signal.c
index 6705320..624a626 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,17 @@ int next_signal(struct sigpending *pending, sigset_t *mask)
 	return sig;
 }

+static void show_reach_rlimit_sigpending(void)
+{
+	DEFINE_RATELIMIT_STATE(printk_rl_state, 5 * HZ, 10);
+
+	if (!__ratelimit(&printk_rl_state))
+		return;
+
+	printk(KERN_INFO "%s/%d: reached RLIMIT_SIGPENDING.\n",
+				current->comm, current->pid);
+}
+
 /*
  * allocate a new signal queue record
  * - this may be called without locks if and only if t == current, otherwise an
@@ -209,8 +222,12 @@ static struct sigqueue *__sigqueue_alloc(struct task_struct
*t, gfp_t flags,
 	atomic_inc(&user->sigpending);
 	if (override_rlimit ||
 	    atomic_read(&user->sigpending) <=
-			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
+			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur) {
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
+	} else {
+		if (print_fatal_signals)
+			show_reach_rlimit_sigpending();
+	}
 	if (unlikely(q == NULL)) {
 		atomic_dec(&user->sigpending);
 		free_uid(user);
@@ -925,8 +942,6 @@ static int send_signal(int sig, struct siginfo *info, struct
task_struct *t,
 	return __send_signal(sig, info, t, group, from_ancestor_ns);
 }

-int print_fatal_signals;
-
 static void print_fatal_signal(struct pt_regs *regs, int signr)
 {
 	printk("%s/%d: potentially unexpected fatal signal %d.\n",

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

[Andrew Morton]

On Sat, 31 Oct 2009 17:50:14 +0900 Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:

> Naohiro Ooiwa wrote:
> > Andrew Morton wrote:
> >> On Fri, 30 Oct 2009 20:36:31 +0900
> >> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> >>>
> >>> +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.
> 
> I fixed my patch.
> Could you please check it.
> 

Please always include the full changelog and signoff with each
iteration of a patch.  That changelog might of course need updating as
the patch changes.


> ---
>  Documentation/kernel-parameters.txt |   11 +++++++++--
>  kernel/signal.c                     |   21 ++++++++++++++++++---
>  2 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> index 9107b38..3bbd92f 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2032,8 +2032,15 @@ and is between 256 and 4096 characters. It is defined in
> the file
> 
>  	print-fatal-signals=
>  			[KNL] debug: print fatal signals
> -			print-fatal-signals=1: print segfault info to
> -			the kernel console.
> +
> +			If enabled, warn about various signal handling
> +			related application anomalies: too many signals,
> +			too many POSIX.1 timers, fatal signals causing a
> +			coredump - etc.
> +
> +			If you hit the warning due to signal overflow,
> +			you might want to try "ulimit -i unlimited".
> +
>  			default: off.
> 
>  	printk.time=	Show timing data prefixed to each printk message line
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 6705320..624a626 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,17 @@ int next_signal(struct sigpending *pending, sigset_t *mask)
>  	return sig;
>  }
> 
> +static void show_reach_rlimit_sigpending(void)
> +{
> +	DEFINE_RATELIMIT_STATE(printk_rl_state, 5 * HZ, 10);

This needs to have `static' storage.  This bug should have been
apparent in your testing?

> +	if (!__ratelimit(&printk_rl_state))
> +		return;
> +
> +	printk(KERN_INFO "%s/%d: reached RLIMIT_SIGPENDING.\n",
> +				current->comm, current->pid);
> +}
> ...
>

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

[Naohiro Ooiwa]

Andrew Morton wrote:
> On Sat, 31 Oct 2009 17:50:14 +0900 Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> 
>> Naohiro Ooiwa wrote:
>>> Andrew Morton wrote:
>>>> On Fri, 30 Oct 2009 20:36:31 +0900
>>>> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> 
> Please always include the full changelog and signoff with each
> iteration of a patch.  That changelog might of course need updating as
> the patch changes.
> 

I'm sorry...
I will be very careful from the next time.

>>
>> +static void show_reach_rlimit_sigpending(void)
>> +{
>> +	DEFINE_RATELIMIT_STATE(printk_rl_state, 5 * HZ, 10);
> 
> This needs to have `static' storage.  This bug should have been
> apparent in your testing?
> 

Again, I'm sorry, I failed to make sure.
But right now, I have test environment.
I tested my patch, result is good.

Thanks you.
Naohiro Ooiwa


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


Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
---
 Documentation/kernel-parameters.txt |   11 +++++++++--
 kernel/signal.c                     |   21 ++++++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 9107b38..3bbd92f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2032,8 +2032,15 @@ and is between 256 and 4096 characters. It is defined in
the file

 	print-fatal-signals=
 			[KNL] debug: print fatal signals
-			print-fatal-signals=1: print segfault info to
-			the kernel console.
+
+			If enabled, warn about various signal handling
+			related application anomalies: too many signals,
+			too many POSIX.1 timers, fatal signals causing a
+			coredump - etc.
+
+			If you hit the warning due to signal overflow,
+			you might want to try "ulimit -i unlimited".
+
 			default: off.

 	printk.time=	Show timing data prefixed to each printk message line
diff --git a/kernel/signal.c b/kernel/signal.c
index 6705320..56e9c00 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,17 @@ int next_signal(struct sigpending *pending, sigset_t *mask)
 	return sig;
 }

+static void show_reach_rlimit_sigpending(void)
+{
+	static DEFINE_RATELIMIT_STATE(printk_rl_state, 5 * HZ, 10);
+
+	if (!__ratelimit(&printk_rl_state))
+		return;
+
+	printk(KERN_INFO "%s/%d: reached RLIMIT_SIGPENDING.\n",
+				current->comm, current->pid);
+}
+
 /*
  * allocate a new signal queue record
  * - this may be called without locks if and only if t == current, otherwise an
@@ -209,8 +222,12 @@ static struct sigqueue *__sigqueue_alloc(struct task_struct
*t, gfp_t flags,
 	atomic_inc(&user->sigpending);
 	if (override_rlimit ||
 	    atomic_read(&user->sigpending) <=
-			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur)
+			t->signal->rlim[RLIMIT_SIGPENDING].rlim_cur) {
 		q = kmem_cache_alloc(sigqueue_cachep, flags);
+	} else {
+		if (print_fatal_signals)
+			show_reach_rlimit_sigpending();
+	}
 	if (unlikely(q == NULL)) {
 		atomic_dec(&user->sigpending);
 		free_uid(user);
@@ -925,8 +942,6 @@ static int send_signal(int sig, struct siginfo *info, struct
task_struct *t,
 	return __send_signal(sig, info, t, group, from_ancestor_ns);
 }

-int print_fatal_signals;
-
 static void print_fatal_signal(struct pt_regs *regs, int signr)
 {
 	printk("%s/%d: potentially unexpected fatal signal %d.\n",
-- 1.5.4.1

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Ingo Molnar]

* Joe Perches <joe@perches.com> wrote:

> Add a printk_ratelimited statement expression macro that uses a 
> per-call ratelimit_state so that multiple subsystems output messages 
> are not suppressed by a global __ratelimit state.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f4e3184..555560c 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -407,6 +407,50 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
>  #endif
>  
>  /*
> + * ratelimited messages with local ratelimit_state,
> + * no local ratelimit_state used in the !PRINTK case
> + */
> +#ifdef CONFIG_PRINTK
> +#define printk_ratelimited(fmt, ...)  ({		\
> +	static struct ratelimit_state _rs = {		\
> +		.interval = DEFAULT_RATELIMIT_INTERVAL, \
> +		.burst = DEFAULT_RATELIMIT_BURST,       \
> +	};                                              \
> +							\
> +	if (!__ratelimit(&_rs))                         \
> +		printk(fmt, ##__VA_ARGS__);		\
> +})
> +#else
> +/* No effect, but we still get type checking even in the !PRINTK case: */
> +#define printk_ratelimited printk
> +#endif
> +
> +#define pr_emerg_rl(fmt, ...) \
> +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_alert_rl(fmt, ...) \
> +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_crit_rl(fmt, ...) \
> +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_err_rl(fmt, ...) \
> +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_warning_rl(fmt, ...) \
> +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_notice_rl(fmt, ...) \
> +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_info_rl(fmt, ...) \
> +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> +/* no pr_cont_rl, don't do that... */
> +/* If you are writing a driver, please use dev_dbg instead */
> +#if defined(DEBUG)
> +#define pr_debug_rl(fmt, ...) \
> +	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#else
> +#define pr_debug_rl(fmt, ...) \
> +	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
> +				     ##__VA_ARGS__); 0; })
> +#endif
> +

Looks like a useful addition. Somewhat bloatier, but then again, more 
correct and the bloat problem can be solved by explicit state 
definitions.

	Ingo

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Naohiro Ooiwa]

Ingo Molnar wrote:
> * Joe Perches <joe@perches.com> wrote:
> 
>> Add a printk_ratelimited statement expression macro that uses a 
>> per-call ratelimit_state so that multiple subsystems output messages 
>> are not suppressed by a global __ratelimit state.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>>
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index f4e3184..555560c 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -407,6 +407,50 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
>>  #endif
>>  
>>  /*
>> + * ratelimited messages with local ratelimit_state,
>> + * no local ratelimit_state used in the !PRINTK case
>> + */
>> +#ifdef CONFIG_PRINTK
>> +#define printk_ratelimited(fmt, ...)  ({		\
>> +	static struct ratelimit_state _rs = {		\
>> +		.interval = DEFAULT_RATELIMIT_INTERVAL, \
>> +		.burst = DEFAULT_RATELIMIT_BURST,       \
>> +	};                                              \
>> +							\
>> +	if (!__ratelimit(&_rs))                         \
>> +		printk(fmt, ##__VA_ARGS__);		\
>> +})
>> +#else
>> +/* No effect, but we still get type checking even in the !PRINTK case: */
>> +#define printk_ratelimited printk
>> +#endif
>> +
>> +#define pr_emerg_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_alert_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_crit_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_err_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_warning_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_notice_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
>> +#define pr_info_rl(fmt, ...) \
>> +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>> +/* no pr_cont_rl, don't do that... */
>> +/* If you are writing a driver, please use dev_dbg instead */
>> +#if defined(DEBUG)
>> +#define pr_debug_rl(fmt, ...) \
>> +	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>> +#else
>> +#define pr_debug_rl(fmt, ...) \
>> +	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
>> +				     ##__VA_ARGS__); 0; })
>> +#endif
>> +
> 
> Looks like a useful addition. Somewhat bloatier, but then again, more 
> correct and the bloat problem can be solved by explicit state 
> definitions.
> 
> 	Ingo

I waiting for this patch to merge.
And then, I think I will remake my patch.

How do you delete printk_ratelimit() in this patch at a same time ?

I have a personal question.
Why aren't they codes in the include/linux/ratelimit.h ?


Thanks.
Naohiro Ooiwa

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Naohiro Ooiwa]

Naohiro Ooiwa wrote:
> Ingo Molnar wrote:
>> * Joe Perches <joe@perches.com> wrote:
>>
>>> Add a printk_ratelimited statement expression macro that uses a 
>>> per-call ratelimit_state so that multiple subsystems output messages 
>>> are not suppressed by a global __ratelimit state.
>>>
>>> Signed-off-by: Joe Perches <joe@perches.com>
>>>
>>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>> index f4e3184..555560c 100644
>>> --- a/include/linux/kernel.h
>>> +++ b/include/linux/kernel.h
>>> @@ -407,6 +407,50 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
>>>  #endif
>>>  
>>>  /*
>>> + * ratelimited messages with local ratelimit_state,
>>> + * no local ratelimit_state used in the !PRINTK case
>>> + */
>>> +#ifdef CONFIG_PRINTK
>>> +#define printk_ratelimited(fmt, ...)  ({		\
>>> +	static struct ratelimit_state _rs = {		\
>>> +		.interval = DEFAULT_RATELIMIT_INTERVAL, \
>>> +		.burst = DEFAULT_RATELIMIT_BURST,       \
>>> +	};                                              \
>>> +							\
>>> +	if (!__ratelimit(&_rs))                         \
>>> +		printk(fmt, ##__VA_ARGS__);		\
>>> +})
>>> +#else
>>> +/* No effect, but we still get type checking even in the !PRINTK case: */
>>> +#define printk_ratelimited printk
>>> +#endif
>>> +
>>> +#define pr_emerg_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_alert_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_crit_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_err_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_warning_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_notice_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_info_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>>> +/* no pr_cont_rl, don't do that... */
>>> +/* If you are writing a driver, please use dev_dbg instead */
>>> +#if defined(DEBUG)
>>> +#define pr_debug_rl(fmt, ...) \
>>> +	printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>>> +#else
>>> +#define pr_debug_rl(fmt, ...) \
>>> +	({ if (0) printk_ratelimited(KERN_DEBUG pr_fmt(fmt), \
>>> +				     ##__VA_ARGS__); 0; })
>>> +#endif
>>> +
>> Looks like a useful addition. Somewhat bloatier, but then again, more 
>> correct and the bloat problem can be solved by explicit state 
>> definitions.
>>
>> 	Ingo
> 
> I waiting for this patch to merge.
> And then, I think I will remake my patch.
> 
> How do you delete printk_ratelimit() in this patch at a same time ?
> 
> I have a personal question.
> Why aren't they codes in the include/linux/ratelimit.h ?
> 

Ah, They are originally in kernel.h . Sorry.

> 
> Thanks.
> Naohiro Ooiwa
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Andrew Morton]

On Fri, 30 Oct 2009 16:21:47 -0700
Joe Perches <joe@perches.com> wrote:

> +#define pr_emerg_rl(fmt, ...) \
> +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_alert_rl(fmt, ...) \
> +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_crit_rl(fmt, ...) \
> +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_err_rl(fmt, ...) \
> +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_warning_rl(fmt, ...) \
> +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_notice_rl(fmt, ...) \
> +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_info_rl(fmt, ...) \
> +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)

Would prefer pr_emerg_ratelimited personally.  It's longer, but one
doesn't ask "wtf does _rl" mean and it avoids having two identifiers
which refer to the same thing.

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Joe Perches]

On Mon, 2009-11-09 at 13:49 -0800, Andrew Morton wrote:
> On Fri, 30 Oct 2009 16:21:47 -0700
> Joe Perches <joe@perches.com> wrote:
> > +#define pr_emerg_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_alert_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_crit_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_err_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_warning_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_notice_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_info_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> 
> Would prefer pr_emerg_ratelimited personally.  It's longer, but one
> doesn't ask "wtf does _rl" mean and it avoids having two identifiers
> which refer to the same thing.

I don't have a strong opinion either way.
_rl is shorter and that has some value.

I think pr_crit_rl, pr_emerg_rl and pr_alert_rl likely
aren't useful.  Is there a sensible use case for those?

I added them for completeness, but...

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Randy Dunlap]

Joe Perches wrote:
> On Mon, 2009-11-09 at 13:49 -0800, Andrew Morton wrote:
>> On Fri, 30 Oct 2009 16:21:47 -0700
>> Joe Perches <joe@perches.com> wrote:
>>> +#define pr_emerg_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_alert_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_crit_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_err_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_warning_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_notice_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
>>> +#define pr_info_rl(fmt, ...) \
>>> +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>> Would prefer pr_emerg_ratelimited personally.  It's longer, but one
>> doesn't ask "wtf does _rl" mean and it avoids having two identifiers
>> which refer to the same thing.
> 
> I don't have a strong opinion either way.
> _rl is shorter and that has some value.

but we have a long history of not using cryptic abbreviations,
so I agree with Andrew.

> I think pr_crit_rl, pr_emerg_rl and pr_alert_rl likely
> aren't useful.  Is there a sensible use case for those?

Not likely.

> I added them for completeness, but...

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 30 Oct 2009 16:21:47 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > +#define pr_emerg_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_alert_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_crit_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_err_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_warning_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_notice_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > +#define pr_info_rl(fmt, ...) \
> > +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> 
> Would prefer pr_emerg_ratelimited personally.  It's longer, but one 
> doesn't ask "wtf does _rl" mean and it avoids having two identifiers 
> which refer to the same thing.

Yeah. It will be rarely used so that it wont ever really be 'obvious at 
a glance', even to folks well versed in kernel source code details.

	Ingo

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Ingo Molnar]

* Joe Perches <joe@perches.com> wrote:

> On Mon, 2009-11-09 at 13:49 -0800, Andrew Morton wrote:
> > On Fri, 30 Oct 2009 16:21:47 -0700
> > Joe Perches <joe@perches.com> wrote:
> > > +#define pr_emerg_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_alert_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_crit_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_err_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_warning_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_notice_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_info_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> > 
> > Would prefer pr_emerg_ratelimited personally.  It's longer, but one
> > doesn't ask "wtf does _rl" mean and it avoids having two identifiers
> > which refer to the same thing.
> 
> I don't have a strong opinion either way.
> _rl is shorter and that has some value.
> 
> I think pr_crit_rl, pr_emerg_rl and pr_alert_rl likely
> aren't useful.  Is there a sensible use case for those?
> 
> I added them for completeness, but...

Yes, we want it for API completeness mostly.

	Ingo

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Peter Zijlstra]

On Tue, 2009-11-10 at 06:17 +0100, Ingo Molnar wrote:
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Fri, 30 Oct 2009 16:21:47 -0700
> > Joe Perches <joe@perches.com> wrote:
> > 
> > > +#define pr_emerg_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_alert_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_crit_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_err_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_warning_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_notice_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > > +#define pr_info_rl(fmt, ...) \
> > > +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> > 
> > Would prefer pr_emerg_ratelimited personally.  It's longer, but one 
> > doesn't ask "wtf does _rl" mean and it avoids having two identifiers 
> > which refer to the same thing.
> 
> Yeah. It will be rarely used so that it wont ever really be 'obvious at 
> a glance', even to folks well versed in kernel source code details.

Is there a reason for all this pr_ nonsense? will we depricate printk()?

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Tue, 2009-11-10 at 06:17 +0100, Ingo Molnar wrote:
> > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Fri, 30 Oct 2009 16:21:47 -0700
> > > Joe Perches <joe@perches.com> wrote:
> > > 
> > > > +#define pr_emerg_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_alert_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_crit_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_err_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_warning_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_notice_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> > > > +#define pr_info_rl(fmt, ...) \
> > > > +        printk_ratelimited(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> > > 
> > > Would prefer pr_emerg_ratelimited personally.  It's longer, but one 
> > > doesn't ask "wtf does _rl" mean and it avoids having two identifiers 
> > > which refer to the same thing.
> > 
> > Yeah. It will be rarely used so that it wont ever really be 'obvious at 
> > a glance', even to folks well versed in kernel source code details.
> 
> Is there a reason for all this pr_ nonsense? will we depricate printk()?

Yes, pr_*() has established itself as a printk shortcut. The benefits 
of:

  pr_info("stuff\n");

versus:

  printk(KERN_INFO "stuff\n");

are sufficiently large:

 - it's shorter by 9 characters (more than a level of indentation)

 - you cannot forget to add a KERN_ prefix - which is required for 98% 
   of all printks but which is forgotten from 50% of the submitted 
   patches.

so pr_*(), while named in a sucky way (all 2 letter abbrevs are sucky), 
has advantages, makes stuff more readable and reduces churn.

printk wont go away as an ad-hoc print-this tool. (Nor will we convert 
most of the remaining 18,000+ uses of printk() in the kernel, so 
printk() will be with us forever i guess.)

	Ingo

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Joe Perches]

On Tue, 2009-11-10 at 08:39 +0100, Ingo Molnar wrote:
> * Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > Is there a reason for all this pr_ nonsense? will we depricate printk()?
> Yes, pr_*() has established itself as a printk shortcut. The benefits 
> of: pr_info("stuff\n"); versus: printk(KERN_INFO "stuff\n");
> are sufficiently large:
>  - it's shorter by 9 characters (more than a level of indentation)
>  - you cannot forget to add a KERN_ prefix - which is required for 98% 
>    of all printks but which is forgotten from 50% of the submitted 
>    patches.
> so pr_*(), while named in a sucky way (all 2 letter abbrevs are sucky), 
> has advantages, makes stuff more readable and reduces churn.

pr_*()s can be prefixed by pr_fmt
pr_*()s could save text space by storing pr_fmt once

Re: [PATCH] kernel.h: Add printk_ratelimited and pr_<level>_rl

[Peter Zijlstra]

On Tue, 2009-11-10 at 08:39 +0100, Ingo Molnar wrote:
> 
> printk wont go away as an ad-hoc print-this tool. (Nor will we convert 
> most of the remaining 18,000+ uses of printk() in the kernel, so 
> printk() will be with us forever i guess.)

Ok good, /me purges all knowledge of pr_* and will henceforth ignore
it ;-)