Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1258490 > unrolled thread

Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj David Rientjes <rientjes@google.com> - 2015-10-29 01:00 +0100
    Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert  from oom_score_adj to oom_adj ebiederm@xmission.com (Eric W. Biederman) - 2015-10-29 05:00 +0100
    Re: 答复: [PATCHv2 4.3-rc6] proc:  fix convert from oom_score_adj to oom_adj Michal Hocko <mhocko@kernel.org> - 2015-10-29 18:10 +0100
      Re: 答复: [PATCHv2 4.3-rc6] proc:  fix convert from oom_score_adj to oom_adj Michal Hocko <mhocko@kernel.org> - 2015-10-30 14:00 +0100
        Re: 答复: [PATCHv2 4.3-rc6] proc:  fix convert from oom_score_adj to oom_adj Michal Hocko <mhocko@kernel.org> - 2015-10-30 15:50 +0100

#1258490 — Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

[Multipart message — attachments visible in raw view] — view raw

On Wed, 28 Oct 2015, Hongjie Fang (方洪杰) wrote:

> Under a userspace perspective, get a different value than he wrote, 
> it must be confusing.
> 

It's confusing, but with purpose: it shows there is no direct mapping 
between /proc/pid/oom_adj and /proc/pid/oom_score_adj.  
/proc/pid/oom_score_adj is the effective policy and has been for years.  
The value returned by /proc/pid/oom_adj demonstrates reality vs what is 
perceived and is a side-effect of integer division truncating the result 
in C.

It's a bad situation, I agree, and we anticipated the complete removal of 
/proc/pid/oom_adj years ago since it has been deprecated for years.  Maybe 
one day we can convince Linus that is possible, but until then we're stuck 
with it.

[toc] | [next] | [standalone]

#1258552 — Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

David Rientjes <rientjes@google.com> writes:

> On Wed, 28 Oct 2015, Hongjie Fang (方洪杰) wrote:
>
>> Under a userspace perspective, get a different value than he wrote, 
>> it must be confusing.
>> 
>
> It's confusing, but with purpose: it shows there is no direct mapping 
> between /proc/pid/oom_adj and /proc/pid/oom_score_adj.  
> /proc/pid/oom_score_adj is the effective policy and has been for years.  
> The value returned by /proc/pid/oom_adj demonstrates reality vs what is 
> perceived and is a side-effect of integer division truncating the result 
> in C.
>
> It's a bad situation, I agree, and we anticipated the complete removal of 
> /proc/pid/oom_adj years ago since it has been deprecated for years.  Maybe 
> one day we can convince Linus that is possible, but until then we're stuck 
> with it.

If you really want to remove /proc/pid/oom_adj start by placing it in a
Kconfig so people can make it go away.

Eric

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

[toc] | [prev] | [next] | [standalone]

#1258920 — Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

On Wed 28-10-15 16:54:04, David Rientjes wrote:
[...]
> It's a bad situation, I agree, and we anticipated the complete removal of 
> /proc/pid/oom_adj years ago since it has been deprecated for years.  Maybe 
> one day we can convince Linus that is possible, but until then we're stuck 
> with it.

Let's do it then.
---
From 71be967d15b3298f3fad7e49ee51f852761b9632 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Thu, 29 Oct 2015 17:42:22 +0100
Subject: [PATCH] oom: get rid of oom_adj

oom_adj has been marked as deprecated by 51b1bd2ace15 ("oom: deprecate
oom_adj tunable") which is quite some time ago. The knob was even
removed by 01dc52ebdf47 ("oom: remove deprecated oom_adj") but
then reintroduced back by fa0cbbf145aa ("mm, oom: reintroduce
/proc/pid/oom_adj") with a scaling mechanism to map oom_adj to
oom_score_adj. The mapping is not ideal and quite confusing as noted by
Hongjie Fang:
$ echo 10 > /proc/1450/oom_adj
$ cat /proc/1450/oom_adj
9

This could be fixed but it seems that the knob has been deprecated for
long enough to finally get rid of it altogether. Most of the reports
happened back in 2012 resp. 2013 and programs were fixed to either
use oom_score_adj or have a fallback mechanism to use oom_adj on older
kernels.

Time has come to finally get rid of this duality and potential source of
confusion. Let's keep OOM_ADJUST_* and OOM_DISABLE in user headers to
not break existing code compilation.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 Documentation/filesystems/proc.txt |  11 +---
 fs/proc/base.c                     | 108 -------------------------------------
 2 files changed, 2 insertions(+), 117 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index d6f259eaa5ef..762bd3f410e9 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -33,8 +33,7 @@ Table of Contents
   2	Modifying System Parameters
 
   3	Per-Process Parameters
-  3.1	/proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj - Adjust the oom-killer
-								score
+  3.1	/proc/<pid>/oom_score_adj - Adjust the oom-killer score
   3.2	/proc/<pid>/oom_score - Display current oom-killer score
   3.3	/proc/<pid>/io - Display the IO accounting fields
   3.4	/proc/<pid>/coredump_filter - Core dump filtering settings
@@ -1436,7 +1435,7 @@ of the kernel.
 CHAPTER 3: PER-PROCESS PARAMETERS
 ------------------------------------------------------------------------------
 
-3.1 /proc/<pid>/oom_adj & /proc/<pid>/oom_score_adj- Adjust the oom-killer score
+3.1 /proc/<pid>/oom_score_adj- Adjust the oom-killer score
 --------------------------------------------------------------------------------
 
 These file can be used to adjust the badness heuristic used to select which
@@ -1477,12 +1476,6 @@ same system, cpuset, mempolicy, or memory controller resources to use at least
 equivalent to discounting 50% of the task's allowed memory from being considered
 as scoring against the task.
 
-For backwards compatibility with previous kernels, /proc/<pid>/oom_adj may also
-be used to tune the badness score.  Its acceptable values range from -16
-(OOM_ADJUST_MIN) to +15 (OOM_ADJUST_MAX) and a special value of -17
-(OOM_DISABLE) to disable oom killing entirely for that task.  Its value is
-scaled linearly with /proc/<pid>/oom_score_adj.
-
 The value of /proc/<pid>/oom_score_adj may be reduced no lower than the last
 value set by a CAP_SYS_RESOURCE process. To reduce the value any lower
 requires CAP_SYS_RESOURCE.
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 60c71b10eaee..f34e43dd8727 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1011,112 +1011,6 @@ static const struct file_operations proc_environ_operations = {
 	.release	= mem_release,
 };
 
-static ssize_t oom_adj_read(struct file *file, char __user *buf, size_t count,
-			    loff_t *ppos)
-{
-	struct task_struct *task = get_proc_task(file_inode(file));
-	char buffer[PROC_NUMBUF];
-	int oom_adj = OOM_ADJUST_MIN;
-	size_t len;
-	unsigned long flags;
-
-	if (!task)
-		return -ESRCH;
-	if (lock_task_sighand(task, &flags)) {
-		if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MAX)
-			oom_adj = OOM_ADJUST_MAX;
-		else
-			oom_adj = (task->signal->oom_score_adj * -OOM_DISABLE) /
-				  OOM_SCORE_ADJ_MAX;
-		unlock_task_sighand(task, &flags);
-	}
-	put_task_struct(task);
-	len = snprintf(buffer, sizeof(buffer), "%d\n", oom_adj);
-	return simple_read_from_buffer(buf, count, ppos, buffer, len);
-}
-
-static ssize_t oom_adj_write(struct file *file, const char __user *buf,
-			     size_t count, loff_t *ppos)
-{
-	struct task_struct *task;
-	char buffer[PROC_NUMBUF];
-	int oom_adj;
-	unsigned long flags;
-	int err;
-
-	memset(buffer, 0, sizeof(buffer));
-	if (count > sizeof(buffer) - 1)
-		count = sizeof(buffer) - 1;
-	if (copy_from_user(buffer, buf, count)) {
-		err = -EFAULT;
-		goto out;
-	}
-
-	err = kstrtoint(strstrip(buffer), 0, &oom_adj);
-	if (err)
-		goto out;
-	if ((oom_adj < OOM_ADJUST_MIN || oom_adj > OOM_ADJUST_MAX) &&
-	     oom_adj != OOM_DISABLE) {
-		err = -EINVAL;
-		goto out;
-	}
-
-	task = get_proc_task(file_inode(file));
-	if (!task) {
-		err = -ESRCH;
-		goto out;
-	}
-
-	task_lock(task);
-	if (!task->mm) {
-		err = -EINVAL;
-		goto err_task_lock;
-	}
-
-	if (!lock_task_sighand(task, &flags)) {
-		err = -ESRCH;
-		goto err_task_lock;
-	}
-
-	/*
-	 * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum
-	 * value is always attainable.
-	 */
-	if (oom_adj == OOM_ADJUST_MAX)
-		oom_adj = OOM_SCORE_ADJ_MAX;
-	else
-		oom_adj = (oom_adj * OOM_SCORE_ADJ_MAX) / -OOM_DISABLE;
-
-	if (oom_adj < task->signal->oom_score_adj &&
-	    !capable(CAP_SYS_RESOURCE)) {
-		err = -EACCES;
-		goto err_sighand;
-	}
-
-	/*
-	 * /proc/pid/oom_adj is provided for legacy purposes, ask users to use
-	 * /proc/pid/oom_score_adj instead.
-	 */
-	pr_warn_once("%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));
-
-	task->signal->oom_score_adj = oom_adj;
-	trace_oom_score_adj_update(task);
-err_sighand:
-	unlock_task_sighand(task, &flags);
-err_task_lock:
-	task_unlock(task);
-	put_task_struct(task);
-out:
-	return err < 0 ? err : count;
-}
-
-static const struct file_operations proc_oom_adj_operations = {
-	.read		= oom_adj_read,
-	.write		= oom_adj_write,
-	.llseek		= generic_file_llseek,
-};
 
 static ssize_t oom_score_adj_read(struct file *file, char __user *buf,
 					size_t count, loff_t *ppos)
@@ -2813,7 +2707,6 @@ static const struct pid_entry tgid_base_stuff[] = {
 	ONE("cgroup",  S_IRUGO, proc_cgroup_show),
 #endif
 	ONE("oom_score",  S_IRUGO, proc_oom_score),
-	REG("oom_adj",    S_IRUGO|S_IWUSR, proc_oom_adj_operations),
 	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
@@ -3161,7 +3054,6 @@ static const struct pid_entry tid_base_stuff[] = {
 	ONE("cgroup",  S_IRUGO, proc_cgroup_show),
 #endif
 	ONE("oom_score", S_IRUGO, proc_oom_score),
-	REG("oom_adj",   S_IRUGO|S_IWUSR, proc_oom_adj_operations),
 	REG("oom_score_adj", S_IRUGO|S_IWUSR, proc_oom_score_adj_operations),
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
-- 
2.6.1



-- 
Michal Hocko
SUSE Labs
--
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/

[toc] | [prev] | [next] | [standalone]

#1259487 — Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

On Thu 29-10-15 18:04:22, Michal Hocko wrote:
> On Wed 28-10-15 16:54:04, David Rientjes wrote:
> [...]
> > It's a bad situation, I agree, and we anticipated the complete removal of 
> > /proc/pid/oom_adj years ago since it has been deprecated for years.  Maybe 
> > one day we can convince Linus that is possible, but until then we're stuck 
> > with it.
> 
> Let's do it then.

I've just checked debian code search and it seems that procps still
relies on oom_adj. I have sent a patch but that sounds like we are not
there yet. I will hunt for other projects still using the deprecated
file exclusively. Hopefully there won't be too many of them.
-- 
Michal Hocko
SUSE Labs
--
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/

[toc] | [prev] | [next] | [standalone]

#1259555 — Re: 答复: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

On Fri 30-10-15 13:59:03, Michal Hocko wrote:
> On Thu 29-10-15 18:04:22, Michal Hocko wrote:
> > On Wed 28-10-15 16:54:04, David Rientjes wrote:
> > [...]
> > > It's a bad situation, I agree, and we anticipated the complete removal of 
> > > /proc/pid/oom_adj years ago since it has been deprecated for years.  Maybe 
> > > one day we can convince Linus that is possible, but until then we're stuck 
> > > with it.
> > 
> > Let's do it then.
> 
> I've just checked debian code search and it seems that procps still
> relies on oom_adj. I have sent a patch but that sounds like we are not
> there yet. I will hunt for other projects still using the deprecated
> file exclusively. Hopefully there won't be too many of them.

It doesn't look that bad afterall:
$ curl -s http://codesearch.debian.net/results/7223e657af3f2ad0/packages.json
{"Packages":["tgt","ggobi","hurd","linux","condor","wine-gecko-2.21","android-platform-frameworks-native","nautilus","procps","wireshark","intel-gpu-tools","iceweasel","icedove","ardour","linux-tools","kde4libs","nss-pam-ldapd","chromium-browser","passenger","archipel-agent-virtualmachine-oomkiller","bleachbit","tilestache","slurm-llnl","ns3","nbd","open-iscsi","mhwaveedit","nilfs-tools","stress-ng","lvm2","gradm2","audit","postgresql-common","zfs-fuse","ocfs2-tools","gimp","advene","lldpad","reniced","pitivi","trinity","petri-foo","rtai","postgresql-9.4","procenv","multipath-tools","percona-toolkit","apparmor","upstart","watchdog","boinc","fusil","util-vserver","booth","geeqie","openssh","oar","android-platform-system-core","kinit","xournal","player","gimp-gap","android-tools"]}

Of those
* android-tools need a trivial patch - not sure who is upstream here
  so pushed through Debian bugzilla - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=803485
* boinc-client.init need a trivial patch - Debian specific it seesm
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=803484
* ocfs2_controld - posted to debian as I wasn't sure about the upstream
  status - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=803486
* procps needs a trivial patch - sent upstream already

This two are nasty because they consume only oom_adj scale so we have
to rescale explicitly :/
* archipel-agent-virtualmachine-oomkiller oom_adj is stored in the DB
* reniced - this is one is nasty as well because it consumes oom_adj
  from user

I will have a look at them early next week.
-- 
Michal Hocko
SUSE Labs
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web