Revert needed: udev spewing warnons on common systems in 3.0

Revert needed: udev spewing warnons on common systems in 3.0

[Andi Kleen]

Hi Linus,

Can you please revert 

commit be8f684d73d8d916847e996bf69cef14352872c6
Author: David Rientjes <rientjes@google.com>
Date:   Mon Jul 25 17:12:18 2011 -0700

    oom: make deprecated use of oom_adj more verbose

    /proc/pid/oom_adj is deprecated and scheduled for removal in August 2012
    according to Documentation/feature-removal-schedule.txt.


This makes most of my test systems (suse 11.1, 11.2) spew scary WARN_ONs
on every boot. GNOME then also complains. While it doesn't cause actual 
misfunction it scares me every time I boot and other people who can't
read git logs like me will be unnecessary scared.

Also the warning is completely useless: noone will be "fixing"
udev on old distributions.

IMHO that's not acceptable to break common user land like this.
Linux is supposed to be binary compatible and this patch is not
in this spirit.

Actually removing the file later should be still fine, but 
not printing out these scary messages.

I propose to revert this misguided patch for stable and 3.1

Thanks,

-Andi


-- 
ak@linux.intel.com -- Speaking for myself only

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Andrew Morton]

On Mon, 1 Aug 2011 17:52:06 -0700 Andi Kleen <andi@firstfloor.org> wrote:

> 
> Hi Linus,
> 
> Can you please revert 
> 
> commit be8f684d73d8d916847e996bf69cef14352872c6
> Author: David Rientjes <rientjes@google.com>
> Date:   Mon Jul 25 17:12:18 2011 -0700
> 
>     oom: make deprecated use of oom_adj more verbose
> 
>     /proc/pid/oom_adj is deprecated and scheduled for removal in August 2012
>     according to Documentation/feature-removal-schedule.txt.
> 
> 
> This makes most of my test systems (suse 11.1, 11.2) spew scary WARN_ONs
> on every boot. GNOME then also complains.

aw, what crap.

"/proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead."

Once per boot.  That's not "scary".

> While it doesn't cause actual 
> misfunction it scares me every time I boot and other people who can't
> read git logs like me will be unnecessary scared.

What?  Look at the damn text - the only reason anyone would need to
read a git log after looking at that is terminal cretinism.

> Also the warning is completely useless: noone will be "fixing"
> udev on old distributions.

I bet there were still old copies of /sbin/update lying around, but we
still managed to remove sys_bdflush() this way.

> IMHO that's not acceptable to break common user land like this.
> Linux is supposed to be binary compatible and this patch is not
> in this spirit.
> 
> Actually removing the file later should be still fine, but 
> not printing out these scary messages.
> 
> I propose to revert this misguided patch for stable and 3.1
> 

I see lots of fake reasoning.  What's really going on here??

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Linus Torvalds]

On Mon, Aug 1, 2011 at 3:32 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> aw, what crap.
>
> "/proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead."
>
> Once per boot.  That's not "scary".

It is when there is that totally pointless stack trace, and the kernel
bug reporting tool picks it up and tells the user that there was a
kernel oops.

Which it really does.

So if it was a "printk_once()" it would be fine. The WARN_ONCE() is
just annoying.

             Linus

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Rientjes]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1466 bytes --]

On Mon, 1 Aug 2011, Linus Torvalds wrote:

> > aw, what crap.
> >
> > "/proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead."
> >
> > Once per boot.  That's not "scary".
> 
> It is when there is that totally pointless stack trace, and the kernel
> bug reporting tool picks it up and tells the user that there was a
> kernel oops.
> 
> Which it really does.
> 
> So if it was a "printk_once()" it would be fine. The WARN_ONCE() is
> just annoying.
> 

It was a printk_once() when the tunable was scheduled to be removed two 
years later.  Now that one year has passed since the oom killer rewrite 
has been merged, it was changed to a WARN_ONCE() simply to attract more 
attention to the issue.  The plan was to remove the tunable completely a 
year from now as specified in Documentation/feature-removal-schedule.txt.

The intent here was to be more helpful than annoying or causing a problem 
with a log parser that doesn't understand the different between a warning 
and a panic or oops.

We could change it to a more obvious multi-line notification that an 
application is using a deprecated interface that wouldn't parse like the 
WARN_ONCE() output if you'd like.  The single printk_once() obviously 
wasn't sufficient over the course of a year to get enough applications to 
change although things like udev, kde, and opensshd did.  chromium 
actually noticed the new WARN_ON() behavior and is in the process of 
fixing it because of that.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Rientjes]

On Mon, 1 Aug 2011, Andi Kleen wrote:

> 
> Hi Linus,
> 
> Can you please revert 
> 
> commit be8f684d73d8d916847e996bf69cef14352872c6
> Author: David Rientjes <rientjes@google.com>
> Date:   Mon Jul 25 17:12:18 2011 -0700
> 
>     oom: make deprecated use of oom_adj more verbose
> 
>     /proc/pid/oom_adj is deprecated and scheduled for removal in August 2012
>     according to Documentation/feature-removal-schedule.txt.
> 
> 
> This makes most of my test systems (suse 11.1, 11.2) spew scary WARN_ONs
> on every boot. GNOME then also complains. While it doesn't cause actual 
> misfunction it scares me every time I boot and other people who can't
> read git logs like me will be unnecessary scared.
> 

If you look at the warning, you should be able to infer that an 
application is writing to /proc/pid/oom_adj when it should be using the 
new /proc/pid/oom_score_adj interface instead.

> Also the warning is completely useless: noone will be "fixing"
> udev on old distributions.
> 

udev was fixed for v162, but admittedly that won't help on old 
distributions.  The WARN_ONCE() was intended to be in place for a year 
just like its previous form, printk_once(), and then the tunable will 
disappear and result in no error message.

> IMHO that's not acceptable to break common user land like this.
> Linux is supposed to be binary compatible and this patch is not
> in this spirit.
> 

Can you show additional breakage that still need to be fixed in userspace 
applications (we can't do anything about old distributions, it'll be a 
silent failure in a year's time)?  udev was fixed for v162, kde was fixed 
for 4.6.1, chromium is in the process of being fixed as a result of this 
WARN_ON() (see http://code.google.com/p/chromium/issues/detail?id=65009), 
and opensshd was fixed in 5.7p1.  oom_adj isn't that common of an 
interface, so if there are others that are largely popular than please let 
me know and we'll get it fixed up.

So I'm certainly not changing an interface and leaving people to fix it 
up, I've been actively involved in doing so for the known userspace that 
does touch the tunable.  I think it's better for users to be notified 
whether by "scary" warnings in the log (come on, we should be able to warn 
about deprecated interfaces in a log without mass failures) as some due 
diligence before removing the tunable.  Most applications use it only to 
disable oom killing entirely, so a silent failure and allowing a vital 
task to be killed is the alternative to getting them fixed up.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Andi Kleen]

David Rientjes <rientjes@google.com> writes:

>
>> Also the warning is completely useless: noone will be "fixing"
>> udev on old distributions.
>> 
>
> udev was fixed for v162, but admittedly that won't help on old 

Nobody wants to update udev if they can avoid it, especially not for
such an extremly obscure reason.

Repeat after me: Linux is not supposed to break old user land.

And not breaking in this case includes not randomly spewing useless
backtraces.

If you feel the need to social engineer the Chromium people, please
do it by email, not by keeping other people's  kernel logs hostage.

> distributions.  The WARN_ONCE() was intended to be in place for a year 
> just like its previous form, printk_once(), and then the tunable will 
> disappear and result in no error message.

printk_once() is fine, but oopsy looking backtraces are not.

>> IMHO that's not acceptable to break common user land like this.
>> Linux is supposed to be binary compatible and this patch is not
>> in this spirit.
>> 
>
> Can you show additional breakage that still need to be fixed in userspace 
> applications (we can't do anything about old distributions, it'll be a 
> silent failure in a year's time)? 

Simply backtraces are not supposed to happen unless something 
is really broken. That's not the case here. The old distribution
works perfectly fine and will continue to do so.

> udev was fixed for v162, kde was fixed 
> for 4.6.1

... and users will continue to run old versions.


> So I'm certainly not changing an interface and leaving people to fix it 
> up, I've been actively involved in doing so for the known userspace that 
> does touch the tunable.  I think it's better for users to be notified 
> whether by "scary" warnings in the log (come on, we should be able to warn 
> about deprecated interfaces in a log without mass failures) as some

You can warn, but not using oopsy looking backtraces. That trigger
all the bug reporting machinery. That is just  annoying.

> diligence before removing the tunable. 

It's not due diligence, it's total overkill for this.

I still think reverting this patch is the right thing to do.

Otherwise I have to do it in my local trees :-/ (it's certainly preferable
than messing with udev)

-Andi


-- 
ak@linux.intel.com -- Speaking for myself only

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Alan Cox]

> It was a printk_once() when the tunable was scheduled to be removed two 
> years later.  Now that one year has passed since the oom killer rewrite 
> has been merged, it was changed to a WARN_ONCE() simply to attract more 
> attention to the issue. 

For the most part the only people whose attention you need are the
distributions not the end users so the WARN isn't helpful.

Alan

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Rientjes]

On Tue, 2 Aug 2011, Alan Cox wrote:

> > It was a printk_once() when the tunable was scheduled to be removed two 
> > years later.  Now that one year has passed since the oom killer rewrite 
> > has been merged, it was changed to a WARN_ONCE() simply to attract more 
> > attention to the issue. 
> 
> For the most part the only people whose attention you need are the
> distributions not the end users so the WARN isn't helpful.
> 

That's obviously wrong since all of the applications that I've cited 
that have been fixed to date have been the result of users actually 
noticing the printk_once() and emailing me directly to pass it along to 
the appropriate parties.  I've also found other cases where bugzilla 
entries were created based on that single line warning by users, who in 
turn get distros and maintainers to convert to the new interface.

It's quite effective, but there are still some applications that have yet 
to be reported, hence the reason for anybody actually hitting this new 
WARN_ONCE().

[patch] oom: change warning for deprecated oom_adj to avoid WARN_ONCE()

[David Rientjes]

On Mon, 1 Aug 2011, Andi Kleen wrote:

> Simply backtraces are not supposed to happen unless something 
> is really broken. That's not the case here. The old distribution
> works perfectly fine and will continue to do so.
> 

It won't work perfectly fine in a year when the tunable is removed 
completely and the code that was writing OOM_DISABLE to /proc/pid/oom_adj 
just fails and the task that was supposed to be prevented from being 
killed at all costs may now be killed.  The printk_once() over the past 
year didn't get that fixed up like the other applications I mentioned did, 
so we need to attract more attention.

> I still think reverting this patch is the right thing to do.
> 

Reverting the patch is ludicrous, otherwise there is little possibility 
that the remaining users of the deprecated interface will change if they 
haven't done so already.  I'm perfectly happy with changing it to a 
different style of warning other than using WARN_ONCE() like I've already 
said.  That doesn't require a revert.

I'm fine with this patch if Linus would like to apply it.


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@firstfloor.org>
Signed-off-by: David Rientjes <rientjes@google.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");
+	}
+
 	task->signal->oom_adj = oom_adjust;
 	/*
 	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Alan Cox]

> That's obviously wrong since all of the applications that I've cited 
> that have been fixed to date have been the result of users actually 
> noticing the printk_once() and emailing me directly to pass it along to 
> the appropriate parties.  I've also found other cases where bugzilla 
> entries were created based on that single line warning by users, who in 
> turn get distros and maintainers to convert to the new interface.

And what did the users do - they complained to the distribution.

> It's quite effective, but there are still some applications that have yet 
> to be reported, hence the reason for anybody actually hitting this new 
> WARN_ONCE().

Perhaps the right way to do it would be to file the bugzillas in the key
distros instead

Actually this raises a more interesting question - would it make sense to
have a moderated notification list for poking all the distributions about
things like old API expiry >

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Miller]

From: David Rientjes <rientjes@google.com>
Date: Tue, 2 Aug 2011 02:53:34 -0700 (PDT)

> That's obviously wrong since all of the applications that I've cited 
> that have been fixed to date have been the result of users actually 
> noticing the printk_once() and emailing me directly to pass it along to 
> the appropriate parties.  I've also found other cases where bugzilla 
> entries were created based on that single line warning by users, who in 
> turn get distros and maintainers to convert to the new interface.
> 
> It's quite effective, but there are still some applications that have yet 
> to be reported, hence the reason for anybody actually hitting this new 
> WARN_ONCE().

I completely agree with you.  Printing out some kind of warning does
get the problems fixed.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Miller]

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Tue, 2 Aug 2011 11:11:17 +0100

> Perhaps the right way to do it would be to file the bugzillas in the key
> distros instead

I completely disagree, there are lots of distributions and filing a bug
with all of them is unreasonable.  Some don't even have bug databases and
are just a bunch of people maintaining a distribution for fun :-)

So even the most obscure distribution might want to notice this and
fix it if it's users care about it enough to report it.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Andi Kleen]

>> That's obviously wrong since all of the applications that I've cited
>> that have been fixed to date have been the result of users actually
>> noticing the printk_once() and emailing me directly to pass it along to
>> the appropriate parties.  I've also found other cases where bugzilla
>> entries were created based on that single line warning by users, who in
>> turn get distros and maintainers to convert to the new interface.
>
> And what did the users do - they complained to the distribution.

I don't think that's how distributions work. Usually they won't fix
an old udev, but just eventually update to a new udev version when
they release a new version of themselves.

But the old release with the old udev will just stay around and people
will continue to use it for many years.

And that's fine because nothing is fundamentally broken here that
needs urgent fixing (except for that bogus backtrace of course)

The kernel has always done a great job in supporting old userland,
this is just one of the rare exceptions.

> Perhaps the right way to do it would be to file the bugzillas in the key
> distros instead

The right way is to add a one linker printk_once and then just wait
for a few years.

> Actually this raises a more interesting question - would it make sense to
> have a moderated notification list for poking all the distributions about
> things like old API expiry

And? Even if the distributions scramble to follow you -- which
is unlikely for old versions -- their users won't do it.

-Andi

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Rientjes]

On Tue, 2 Aug 2011, Alan Cox wrote:

> > That's obviously wrong since all of the applications that I've cited 
> > that have been fixed to date have been the result of users actually 
> > noticing the printk_once() and emailing me directly to pass it along to 
> > the appropriate parties.  I've also found other cases where bugzilla 
> > entries were created based on that single line warning by users, who in 
> > turn get distros and maintainers to convert to the new interface.
> 
> And what did the users do - they complained to the distribution.
> 

All of the examples that I've cited that were fixed as a result of the 
printk_once() were through bugzilla for the developers of those 
applications, I didn't talk to any distro when I publically said I would 
handle all reports of this warning that people didn't know how to triage.

> Actually this raises a more interesting question - would it make sense to
> have a moderated notification list for poking all the distributions about
> things like old API expiry >
> 

I think that's the goal of linux-api@vger.kernel.org although it only has 
113 subscribers, so it could probably play the role of a notification list 
as well as a place to report warnings for things such as deprecated 
interfaces.  More communication other than just sometimes stale entries in 
Documentation/ABI and Documentation/feature-removal-schedule.txt would 
certainly be helpful.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[David Rientjes]

On Tue, 2 Aug 2011, Andi Kleen wrote:

> And that's fine because nothing is fundamentally broken here that
> needs urgent fixing (except for that bogus backtrace of course)
> 

Maybe not urgent, but needs to be fixed within the next 12 months because 
all of the users of /proc/pid/oom_adj thus far have been to disable oom 
killing for the thread and that protection isn't guaranteed when the 
tunable is removed.  Doing echo -17 > /proc/pid/oom_adj is a pretty good 
indicator that the process is important.

> The right way is to add a one linker printk_once and then just wait
> for a few years.
> 

This is what I did, except I waited one year instead of a few and now I 
wanted to add a more verbose warning for those that still haven't been 
fixed because we're one year away from the tunable's scheduled removal.  
If you're going to reply with a concern about the stack trace again, 
please see my patch I posted for Linus if the kernel log parsers are 
really that delicate.

Re: Revert needed: udev spewing warnons on common systems in 3.0

[Andi Kleen]

> All of the examples that I've cited that were fixed as a result of the 
> printk_once() were through bugzilla for the developers of those 
> applications, I didn't talk to any distro when I publically said I would 
> handle all reports of this warning that people didn't know how to triage.

The distros would have updated udev at some point anyways.
But they won't do it for old releases.

But the users who don't want to update will just be annoyed 
(in Linus words) the backtrace forever. 

The big problem I have with your "use kernel WARN_ONs to social engineer"
approach is that the devalues a good mechanism to report kernel 
problems.

Traditionally at least I looked at all backtraces
and tried to get at the bottom of the problem to improve the kernel.
I know from email that users reported those too.

Now that all my test systems spew them on every boot I cannot do 
that anymore. When I see a backtrace scrolling by I now just
think "Ah it's probably David Rientjes trying to social engineer
someone again". 

And then some point noone will look at backtraces anymore because
they have become normal.  Really serious problems will get missed.

And this is a bad thing.  

IMHO still the only sensible thing that can be done with this patch
is to revert it.

-Andi

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

[Borislav Petkov]

On Tue, Aug 02, 2011 at 02:54:01AM -0700, David Rientjes wrote:
> On Mon, 1 Aug 2011, Andi Kleen wrote:
> 
> > Simply backtraces are not supposed to happen unless something 
> > is really broken. That's not the case here. The old distribution
> > works perfectly fine and will continue to do so.
> > 
> 
> It won't work perfectly fine in a year when the tunable is removed 
> completely and the code that was writing OOM_DISABLE to /proc/pid/oom_adj 
> just fails and the task that was supposed to be prevented from being 
> killed at all costs may now be killed.  The printk_once() over the past 
> year didn't get that fixed up like the other applications I mentioned did, 
> so we need to attract more attention.
> 
> > I still think reverting this patch is the right thing to do.
> > 
> 
> Reverting the patch is ludicrous, otherwise there is little possibility 
> that the remaining users of the deprecated interface will change if they 
> haven't done so already.  I'm perfectly happy with changing it to a 
> different style of warning other than using WARN_ONCE() like I've already 
> said.  That doesn't require a revert.
> 
> I'm fine with this patch if Linus would like to apply it.
> 
> 
> 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@firstfloor.org>
> Signed-off-by: David Rientjes <rientjes@google.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. 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.

-- 
Regards/Gruss,
    Boris.

Re: [patch] oom: change warning for deprecated oom_adj to avoid WARN_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@firstfloor.org>
> > Signed-off-by: David Rientjes <rientjes@google.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.

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

[Borislav Petkov]

On Wed, Aug 03, 2011 at 11:04:55PM -0700, David Rientjes wrote:
> > > -	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.

I see.

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

Or maybe do:

	pr_info_once("\n===============================================================================\n"
		     "%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n"
		     "===============================================================================\n\n",
		     current->comm, task_pid_nr(current), task_pid_nr(task), task_pid_nr(task));

so as to not be a warning and still hide the warning_printed thing. You
could even save yourself the last repeated argument by enumerating the
format specifiers:

	pr_info_once("\n===============================================================================\n"
		     "%1$s (%2$d): /proc/%3$d/oom_adj is deprecated, please use /proc/%3$d/oom_score_adj instead.\n"
		     "===============================================================================\n\n",
		     current->comm, task_pid_nr(current), task_pid_nr(task));

It seems to build fine here.

Oh well. :-)

-- 
Regards/Gruss,
    Boris.