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

Groups > linux.kernel > #1253522 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj Hongjie Fang (方洪杰)   <Hongjie.Fang@spreadtrum.com> - 2015-10-22 09:00 +0200
    Re: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj Michal Hocko <mhocko@kernel.org> - 2015-10-22 11:50 +0200
    Re: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to  oom_adj David Rientjes <rientjes@google.com> - 2015-10-26 22:50 +0100
      Re: [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj Michal Hocko <mhocko@kernel.org> - 2015-10-27 13:40 +0100

#1253522 — [PATCHv2 4.3-rc6] proc: fix convert from oom_score_adj to oom_adj

The oom_adj has been replaced by oom_score_adj in kernel,
but the /proc/pid/oom_adj is provided for legacy purposes.
When write/read a value into/from /proc/pid/oom_adj,
there is a transformation between oom_adj and oom_score_adj.

After writing a new value into /proc/pid/oom_adj, then read it.
The return value is a different value than you wrote.
Fix this by adding a adjustment factor.

Signed-off-by: Hongjie Fang <hongjie.fang@spreadtrum.com>
---
Encountered the problem when I changed a task's oom_adj on
an Android smart phone. As follows,
1. # cat /proc/1450/oom_adj
   15
2. # echo 10 > /proc/1450/oom_adj
3. # cat /proc/1450/oom_adj
   9


diff --git a/fs/proc/base.c b/fs/proc/base.c
index b25eee4..2312e43 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1019,15 +1019,19 @@ static ssize_t oom_adj_read(struct file *file, char __user *buf, size_t count,
 	int oom_adj = OOM_ADJUST_MIN;
 	size_t len;
 	unsigned long flags;
+	int adjust;
 
 	if (!task)
 		return -ESRCH;
 	if (lock_task_sighand(task, &flags)) {
-		if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MAX)
+		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) /
+		} else {
+			adjust = task->signal->oom_score_adj > 0 ?
+				 (OOM_SCORE_ADJ_MAX-1) : -(OOM_SCORE_ADJ_MAX-1);
+			oom_adj = (task->signal->oom_score_adj * -OOM_DISABLE + adjust) /
 				  OOM_SCORE_ADJ_MAX;
+		}
 		unlock_task_sighand(task, &flags);
 	}
 	put_task_struct(task);
--
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] | [next] | [standalone]

#1253679

On Thu 22-10-15 06:49:01, Hongjie Fang (方洪杰) wrote:
> 
> The oom_adj has been replaced by oom_score_adj in kernel,
> but the /proc/pid/oom_adj is provided for legacy purposes.
> When write/read a value into/from /proc/pid/oom_adj,
> there is a transformation between oom_adj and oom_score_adj.
> 
> After writing a new value into /proc/pid/oom_adj, then read it.
> The return value is a different value than you wrote.
> Fix this by adding a adjustment factor.

... when printing the value.

Your previous patch has changed the stored value while this one only
changes the presented value. The previous one was more correct IMO
but in reality the difference (+-1) in oom_score_adj should be hardly
noticeable and nobody has complained about the current scaling for years
so this approach is probably more conservative.

> Signed-off-by: Hongjie Fang <hongjie.fang@spreadtrum.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
> Encountered the problem when I changed a task's oom_adj on
> an Android smart phone. As follows,
> 1. # cat /proc/1450/oom_adj
>    15
> 2. # echo 10 > /proc/1450/oom_adj
> 3. # cat /proc/1450/oom_adj
>    9
> 
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index b25eee4..2312e43 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1019,15 +1019,19 @@ static ssize_t oom_adj_read(struct file *file, char __user *buf, size_t count,
>  	int oom_adj = OOM_ADJUST_MIN;
>  	size_t len;
>  	unsigned long flags;
> +	int adjust;
>  
>  	if (!task)
>  		return -ESRCH;
>  	if (lock_task_sighand(task, &flags)) {
> -		if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MAX)
> +		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) /
> +		} else {
> +			adjust = task->signal->oom_score_adj > 0 ?
> +				 (OOM_SCORE_ADJ_MAX-1) : -(OOM_SCORE_ADJ_MAX-1);
> +			oom_adj = (task->signal->oom_score_adj * -OOM_DISABLE + adjust) /
>  				  OOM_SCORE_ADJ_MAX;
> +		}
>  		unlock_task_sighand(task, &flags);
>  	}
>  	put_task_struct(task);
> --
> 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/

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

#1256307 — 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 Thu, 22 Oct 2015, Hongjie Fang (方洪杰) wrote:

> 
> The oom_adj has been replaced by oom_score_adj in kernel,
> but the /proc/pid/oom_adj is provided for legacy purposes.
> When write/read a value into/from /proc/pid/oom_adj,
> there is a transformation between oom_adj and oom_score_adj.
> 
> After writing a new value into /proc/pid/oom_adj, then read it.
> The return value is a different value than you wrote.
> Fix this by adding a adjustment factor.
> 

You're only looking at the output and seeing that it disagrees with what 
was written and ignoring _why_ it disagrees.

It's because, as I already stated, oom_score_adj is the effective tunable 
for oom kill process prioritization and the legacy oom_adj had a different 
scale where a 1:1 mapping is not possible.

All throughout the kernel, we report the effective value.  We accept 
writes and the reads report the effective value.  This is no different.

Nack again.

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

#1256727

On Mon 26-10-15 14:42:57, David Rientjes wrote:
> On Thu, 22 Oct 2015, Hongjie Fang (方洪杰) wrote:
> 
> > 
> > The oom_adj has been replaced by oom_score_adj in kernel,
> > but the /proc/pid/oom_adj is provided for legacy purposes.
> > When write/read a value into/from /proc/pid/oom_adj,
> > there is a transformation between oom_adj and oom_score_adj.
> > 
> > After writing a new value into /proc/pid/oom_adj, then read it.
> > The return value is a different value than you wrote.
> > Fix this by adding a adjustment factor.
> > 
> 
> You're only looking at the output and seeing that it disagrees with what 
> was written and ignoring _why_ it disagrees.
> 
> It's because, as I already stated, oom_score_adj is the effective tunable 
> for oom kill process prioritization and the legacy oom_adj had a different 
> scale where a 1:1 mapping is not possible.
> 
> All throughout the kernel, we report the effective value.  We accept 
> writes and the reads report the effective value.  This is no different.
> 
> Nack again.

I really fail to understand your reasoning. The patch basically fixes up
the presented value of oom_adj after rounding imprecision. It doesn't
change the way how the oom_adj->oom_score_aj mapping is done at all. All
it does is that it presents oom_adj1 -> oom_score_adj -> oom_adj2 and
oom_adj1 = oom_adj2

How can this be any harmful? And more importantly why do you want to
expose the imprecision in the mapping to the user space in the first
place?
-- 
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