linux-kernel - Re: [patch] oom: change warning for deprecated oom_adj to avoid WARN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.00.1108032258200.23444@chino.kir.corp.google.com>
Date:	Wed, 3 Aug 2011 23:04:55 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Borislav Petkov <bp@...en8.de>, Andi Kleen <andi@...stfloor.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [patch] oom: change warning for deprecated oom_adj to avoid
 WARN_ONCE()

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@...stfloor.org>
> > Signed-off-by: David Rientjes <rientjes@...gle.com>
> > ---
> >  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/