[PATCH] OOM killer meets userspace headers

[PATCH] OOM killer meets userspace headers

[Alexey Dobriyan]

Despite mm.h is not being exported header, it does contain one thing
which is part of userspace ABI -- value disabling OOM killer. So,
a) export mm.h to userspace
b) got OOM_DISABLE disable define out of __KERNEL__ prison.
c) turn bound values suitable for /proc/$PID/oom_adj into defines and export
   them too.
d) put some headers into __KERNEL__ prison. It'd bizarre to include mm.h and
   get capability stuff.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/proc/base.c       |    3 ++-
 include/linux/Kbuild |    2 ++
 include/linux/mm.h   |   13 +++++++------
 3 files changed, 11 insertions(+), 7 deletions(-)

--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -689,7 +689,8 @@ static ssize_t oom_adjust_write(struct f
 	if (copy_from_user(buffer, buf, count))
 		return -EFAULT;
 	oom_adjust = simple_strtol(buffer, &end, 0);
-	if ((oom_adjust < -16 || oom_adjust > 15) && oom_adjust != OOM_DISABLE)
+	if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) &&
+	     oom_adjust != OOM_DISABLE)
 		return -EINVAL;
 	if (*end == '\n')
 		end++;
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -110,6 +110,7 @@ header-y += major.h
 header-y += matroxfb.h
 header-y += meye.h
 header-y += minix_fs.h
+header-y += mm.h
 header-y += mmtimer.h
 header-y += mqueue.h
 header-y += mtio.h
@@ -257,6 +258,7 @@ unifdef-y += loop.h
 unifdef-y += lp.h
 unifdef-y += mempolicy.h
 unifdef-y += mii.h
+unifdef-y += mm.h
 unifdef-y += mman.h
 unifdef-y += mroute.h
 unifdef-y += msdos_fs.h
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1,12 +1,16 @@
 #ifndef _LINUX_MM_H
 #define _LINUX_MM_H
 
+/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
+#define OOM_DISABLE (-17)
+/* inclusive */
+#define OOM_ADJUST_MIN (-16)
+#define OOM_ADJUST_MAX 15
+
+#ifdef __KERNEL__
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <linux/capability.h>
-
-#ifdef __KERNEL__
-
 #include <linux/gfp.h>
 #include <linux/list.h>
 #include <linux/mmzone.h>
@@ -1115,9 +1119,6 @@ int in_gate_area_no_task(unsigned long a
 #define in_gate_area(task, addr) ({(void)task; in_gate_area_no_task(addr);})
 #endif	/* __HAVE_ARCH_GATE_AREA */
 
-/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
-#define OOM_DISABLE -17
-
 int drop_caches_sysctl_handler(struct ctl_table *, int, struct file *,
 					void __user *, size_t *, loff_t *);
 unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,

Re: [PATCH] OOM killer meets userspace headers

[Nick Piggin]

Alexey Dobriyan wrote:
> Despite mm.h is not being exported header, it does contain one thing
> which is part of userspace ABI -- value disabling OOM killer. So,
> a) export mm.h to userspace
> b) got OOM_DISABLE disable define out of __KERNEL__ prison.
> c) turn bound values suitable for /proc/$PID/oom_adj into defines and export
>    them too.
> d) put some headers into __KERNEL__ prison. It'd bizarre to include mm.h and
>    get capability stuff.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---

Seems fine. Silly question:

> +/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
> +#define OOM_DISABLE (-17)
> +/* inclusive */
> +#define OOM_ADJUST_MIN (-16)
> +#define OOM_ADJUST_MAX 15

Why do you need the () for the -ves?

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [PATCH] OOM killer meets userspace headers

[David Woodhouse]

On Wed, 2006-10-18 at 18:53 +0400, Alexey Dobriyan wrote:
> Despite mm.h is not being exported header, it does contain one thing
> which is part of userspace ABI -- value disabling OOM killer. So,
> a) export mm.h to userspace

You seem to be adding it _both_ to $(header-y) which makes it get
exported without using unifdef, and to $(unifdef-y) which makes it get
exported _with_ unifdef. Choose one or the other.

Other than that, it looks Ok.

-- 
dwmw2

Re: [PATCH] OOM killer meets userspace headers

[Christoph Hellwig]

On Wed, Oct 18, 2006 at 06:53:05PM +0400, Alexey Dobriyan wrote:
> Despite mm.h is not being exported header, it does contain one thing
> which is part of userspace ABI -- value disabling OOM killer. So,
> a) export mm.h to userspace
> b) got OOM_DISABLE disable define out of __KERNEL__ prison.
> c) turn bound values suitable for /proc/$PID/oom_adj into defines and export
>    them too.
> d) put some headers into __KERNEL__ prison. It'd bizarre to include mm.h and
>    get capability stuff.

NACK, mm.h is far too big for that.  Just create a tiny oom.h for those
values.

[PATCH v2] OOM killer meets userspace headers

[Alexey Dobriyan]

On Wed, Oct 18, 2006 at 04:09:48PM +0100, Christoph Hellwig wrote:
> On Wed, Oct 18, 2006 at 06:53:05PM +0400, Alexey Dobriyan wrote:
> > Despite mm.h is not being exported header, it does contain one thing
> > which is part of userspace ABI -- value disabling OOM killer. So,
> > a) export mm.h to userspace
> > b) got OOM_DISABLE disable define out of __KERNEL__ prison.
> > c) turn bound values suitable for /proc/$PID/oom_adj into defines and export
> >    them too.
> > d) put some headers into __KERNEL__ prison. It'd bizarre to include mm.h and
> >    get capability stuff.
>
> NACK, mm.h is far too big for that.  Just create a tiny oom.h for those
> values.

It _will_ be tiny after "make headers_install". OTOH, oom.h indeed looks
like logical place.


[PATCH v2] OOM killer meets userspace headers

Despite mm.h is not being exported header, it does contain one thing
which is part of userspace ABI -- value disabling OOM killer for given
process. So,
a) create and export include/linux/oom.h
b) move OOM_DISABLE define there.
c) turn bounding values of /proc/$PID/oom_adj into defines and export
   them too.

Note: mass __KERNEL__ removal will be done later.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/proc/base.c       |    4 +++-
 include/linux/Kbuild |    1 +
 include/linux/mm.h   |    3 ---
 include/linux/oom.h  |   10 ++++++++++
 mm/oom_kill.c        |    1 +
 5 files changed, 15 insertions(+), 4 deletions(-)

--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -72,6 +72,7 @@ #include <linux/cpuset.h>
 #include <linux/audit.h>
 #include <linux/poll.h>
 #include <linux/nsproxy.h>
+#include <linux/oom.h>
 #include "internal.h"
 
 /* NOTE:
@@ -689,7 +690,8 @@ static ssize_t oom_adjust_write(struct f
 	if (copy_from_user(buffer, buf, count))
 		return -EFAULT;
 	oom_adjust = simple_strtol(buffer, &end, 0);
-	if ((oom_adjust < -16 || oom_adjust > 15) && oom_adjust != OOM_DISABLE)
+	if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) &&
+	     oom_adjust != OOM_DISABLE)
 		return -EINVAL;
 	if (*end == '\n')
 		end++;
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -120,6 +120,7 @@ header-y += netrom.h
 header-y += nfs2.h
 header-y += nfs4_mount.h
 header-y += nfs_mount.h
+header-y += oom.h
 header-y += param.h
 header-y += pci_ids.h
 header-y += pci_regs.h
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1115,9 +1115,6 @@ int in_gate_area_no_task(unsigned long a
 #define in_gate_area(task, addr) ({(void)task; in_gate_area_no_task(addr);})
 #endif	/* __HAVE_ARCH_GATE_AREA */
 
-/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
-#define OOM_DISABLE -17
-
 int drop_caches_sysctl_handler(struct ctl_table *, int, struct file *,
 					void __user *, size_t *, loff_t *);
 unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,
--- /dev/null
+++ b/include/linux/oom.h
@@ -0,0 +1,10 @@
+#ifndef __INCLUDE_LINUX_OOM_H
+#define __INCLUDE_LINUX_OOM_H
+
+/* /proc/<pid>/oom_adj set to -17 protects from the oom-killer */
+#define OOM_DISABLE (-17)
+/* inclusive */
+#define OOM_ADJUST_MIN (-16)
+#define OOM_ADJUST_MAX 15
+
+#endif
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -15,6 +15,7 @@
  *  kernel subsystems and hints as to where to find out what things do.
  */
 
+#include <linux/oom.h>
 #include <linux/mm.h>
 #include <linux/sched.h>
 #include <linux/swap.h>

Re: [KJ] [PATCH] OOM killer meets userspace headers

[Alexey Dobriyan]

On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> >+#define OOM_ADJUST_MIN (-16)
> >+#define OOM_ADJUST_MAX 15
>
> Why do you need the () for the -ves?

-16 is two tokens. Not that someone is going to do huge arithmetic with
OOM adjustments and screwup himself, but still...
15 is one token, so it's safe.

And if someone from kernel-janitors would go through tree and (without
fanatism) add additional parenthesis to where they belong, it would be
nice.

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [PATCH] OOM killer meets userspace headers

[Alexey Dobriyan]

On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> >+#define OOM_ADJUST_MIN (-16)
> >+#define OOM_ADJUST_MAX 15
>
> Why do you need the () for the -ves?

-16 is two tokens. Not that someone is going to do huge arithmetic with
OOM adjustments and screwup himself, but still...
15 is one token, so it's safe.

And if someone from kernel-janitors would go through tree and (without
fanatism) add additional parenthesis to where they belong, it would be
nice.

Re: [KJ] [PATCH] OOM killer meets userspace headers

[Nick Piggin]

Alexey Dobriyan wrote:
> On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> 
>>>+#define OOM_ADJUST_MIN (-16)
>>>+#define OOM_ADJUST_MAX 15
>>
>>Why do you need the () for the -ves?
> 
> 
> -16 is two tokens. Not that someone is going to do huge arithmetic with
> OOM adjustments and screwup himself, but still...

How can they screw themselves up? AFAIKS, the - directly to the left
of the literal will bind more tightly than any other valid operator.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [PATCH] OOM killer meets userspace headers

[Nick Piggin]

Alexey Dobriyan wrote:
> On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> 
>>>+#define OOM_ADJUST_MIN (-16)
>>>+#define OOM_ADJUST_MAX 15
>>
>>Why do you need the () for the -ves?
> 
> 
> -16 is two tokens. Not that someone is going to do huge arithmetic with
> OOM adjustments and screwup himself, but still...

How can they screw themselves up? AFAIKS, the - directly to the left
of the literal will bind more tightly than any other valid operator.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [KJ] [PATCH] OOM killer meets userspace headers

[Alexey Dobriyan]

On Thu, Oct 19, 2006 at 05:12:19AM +1000, Nick Piggin wrote:
> Alexey Dobriyan wrote:
> >On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> >
> >>>+#define OOM_ADJUST_MIN (-16)
> >>>+#define OOM_ADJUST_MAX 15
> >>
> >>Why do you need the () for the -ves?
> >
> >
> >-16 is two tokens. Not that someone is going to do huge arithmetic with
> >OOM adjustments and screwup himself, but still...
> 
> How can they screw themselves up? AFAIKS, the - directly to the left
> of the literal will bind more tightly than any other valid operator.

Hmmm... c.l.c lists two reasons: a) =- being synonym of -= in pre-ANSI
days, and b) fat fingers

	#define EOF -1
	while ((c = getchar()) != 3 EOF)

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [PATCH] OOM killer meets userspace headers

[Alexey Dobriyan]

On Thu, Oct 19, 2006 at 05:12:19AM +1000, Nick Piggin wrote:
> Alexey Dobriyan wrote:
> >On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
> >
> >>>+#define OOM_ADJUST_MIN (-16)
> >>>+#define OOM_ADJUST_MAX 15
> >>
> >>Why do you need the () for the -ves?
> >
> >
> >-16 is two tokens. Not that someone is going to do huge arithmetic with
> >OOM adjustments and screwup himself, but still...
> 
> How can they screw themselves up? AFAIKS, the - directly to the left
> of the literal will bind more tightly than any other valid operator.

Hmmm... c.l.c lists two reasons: a) =- being synonym of -= in pre-ANSI
days, and b) fat fingers

	#define EOF -1
	while ((c = getchar()) != 3 EOF)

Re: [KJ] [PATCH] OOM killer meets userspace headers

[Nick Piggin]

Alexey Dobriyan wrote:
> On Thu, Oct 19, 2006 at 05:12:19AM +1000, Nick Piggin wrote:
> 
>>Alexey Dobriyan wrote:
>>
>>>On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
>>>
>>>
>>>>>+#define OOM_ADJUST_MIN (-16)
>>>>>+#define OOM_ADJUST_MAX 15
>>>>
>>>>Why do you need the () for the -ves?
>>>
>>>
>>>-16 is two tokens. Not that someone is going to do huge arithmetic with
>>>OOM adjustments and screwup himself, but still...
>>
>>How can they screw themselves up? AFAIKS, the - directly to the left
>>of the literal will bind more tightly than any other valid operator.
> 
> 
> Hmmm... c.l.c lists two reasons: a) =- being synonym of -= in pre-ANSI
> days, and b) fat fingers
> 
> 	#define EOF -1
> 	while ((c = getchar()) != 3 EOF)

I can't say I care about those problems to justify the uglification
(or churning the tree).

If the operator were legitimately able to leak out, obviously () is
a good thing. Otherwise...

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [PATCH] OOM killer meets userspace headers

[Nick Piggin]

Alexey Dobriyan wrote:
> On Thu, Oct 19, 2006 at 05:12:19AM +1000, Nick Piggin wrote:
> 
>>Alexey Dobriyan wrote:
>>
>>>On Thu, Oct 19, 2006 at 01:05:53AM +1000, Nick Piggin wrote:
>>>
>>>
>>>>>+#define OOM_ADJUST_MIN (-16)
>>>>>+#define OOM_ADJUST_MAX 15
>>>>
>>>>Why do you need the () for the -ves?
>>>
>>>
>>>-16 is two tokens. Not that someone is going to do huge arithmetic with
>>>OOM adjustments and screwup himself, but still...
>>
>>How can they screw themselves up? AFAIKS, the - directly to the left
>>of the literal will bind more tightly than any other valid operator.
> 
> 
> Hmmm... c.l.c lists two reasons: a) =- being synonym of -= in pre-ANSI
> days, and b) fat fingers
> 
> 	#define EOF -1
> 	while ((c = getchar()) != 3 EOF)

I can't say I care about those problems to justify the uglification
(or churning the tree).

If the operator were legitimately able to leak out, obviously () is
a good thing. Otherwise...

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com