Re: [patch] oom: change warning for deprecated oom_adj to avoidWARN_ONCE()

[David Rientjes]

On Wed, 3 Aug 2011, Borislav Petkov wrote:

> > oom: change warning for deprecated oom_adj to avoid WARN_ONCE()
> > 
> > WARN_ONCE() emits a stack trace to the kernel log which leads userspace 
> > parsers to interpret it as being a serious error or malfunction within the 
> > kernel.  Change the warning to appear more like a lockdep warning while 
> > still trying to preserve the intention of be8f684d73d8 (oom: make 
> > deprecated use of oom_adj more verbose) to attract more attention to the 
> > use of a deprecated interface.
> > 
> > Reported-by: Andi Kleen <andi@xxxxxxxxxxxxxx>
> > Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
> > ---
> >  fs/proc/base.c |   13 ++++++++++---
> >  1 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -1066,6 +1066,7 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
> >  	char buffer[PROC_NUMBUF];
> >  	int oom_adjust;
> >  	unsigned long flags;
> > +	static bool warning_printed;
> >  	int err;
> >  
> >  	memset(buffer, 0, sizeof(buffer));
> > @@ -1118,9 +1119,15 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf,
> >  	 * Warn that /proc/pid/oom_adj is deprecated, see
> >  	 * Documentation/feature-removal-schedule.txt.
> >  	 */
> > -	WARN_ONCE(1, "%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
> > -		  current->comm, task_pid_nr(current), task_pid_nr(task),
> > -		  task_pid_nr(task));
> > +	if (!warning_printed) {
> > +		warning_printed = true;
> > +		printk("\n===============================================================================\n");
> > +		printk("%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
> > +			current->comm, task_pid_nr(current), task_pid_nr(task),
> > +			task_pid_nr(task));
> > +		printk("===============================================================================\n\n");
> 
> You're missing the KERN_WARNING level.

It's intentional because (i) I'm using a multi-line notification with 
newlines and (ii) I don't want to be considered as a kernel warning.  It's 
just for consumption by userspace and doesn't indicate a kernel issue.

> Why don't you use pr_warn_once +
> pr_cont_once? No need for the warning_printed too, it gets defined in
> another scope by the pr_warn_once macro automatically.
> 

Because using pr_warn_once + pr_cont_once for a multi-line notification is 
racy and I don't want three separated static variables?  pr_cont_once() 
shouldn't be used unless synchronized.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/