[PATCH] bitbake.conf: update way of setting XZ_MEMLIMIT

[PATCH] bitbake.conf: update way of setting XZ_MEMLIMIT

[Changqing Li]

From: Changqing Li <changqing.li@windriver.com>

Update way of setting XZ_MEMLIMIT, considering scenario that
running bitbake in container, to avoid OOM Killer of xz.

For example:
Container has memory limit of 30G, and host memory is 300G.  'xz
--memlimit=50% ...' will get host memory, so the limit for xz is 150G.
And because of the container memory limit is 30G, xz can use nearly up
to 30G memory, which may cause oom kill of xz.

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 meta/conf/bitbake.conf |  3 ++-
 meta/lib/oe/utils.py   | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index f6fb2aa698..2b36e083ca 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -809,7 +809,8 @@ BB_NUMBER_THREADS ?= "${@oe.utils.cpu_count()}"
 PARALLEL_MAKE ?= "-j ${@oe.utils.cpu_count()}"
 
 # Default parallelism and resource usage for xz
-XZ_MEMLIMIT ?= "50%"
+CONTAINER_MEM_LIMIT = "${@oe.utils.container_mem_limit()}"
+XZ_MEMLIMIT ?= "${@ '%d' % (int(d.getVar('CONTAINER_MEM_LIMIT'))/2) if d.getVar('CONTAINER_MEM_LIMIT') != '0' else '50%'}"
 XZ_THREADS ?= "${@oe.utils.cpu_count(at_least=2)}"
 XZ_DEFAULTS ?= "--memlimit=${XZ_MEMLIMIT} --threads=${XZ_THREADS}"
 XZ_DEFAULTS[vardepsexclude] += "XZ_MEMLIMIT XZ_THREADS"
diff --git a/meta/lib/oe/utils.py b/meta/lib/oe/utils.py
index a84039f585..581072ac0d 100644
--- a/meta/lib/oe/utils.py
+++ b/meta/lib/oe/utils.py
@@ -252,6 +252,18 @@ def cpu_count(at_least=1):
     cpus = len(os.sched_getaffinity(0))
     return max(cpus, at_least)
 
+def container_mem_limit():
+    limit_in_bytes = '0'
+    proc_sched = '/proc/1/sched'
+    if os.path.exists(proc_sched):
+        with open(proc_sched, 'r') as fp:
+            initinfo = fp.readline().strip()
+            cgroup_limit_in_bytes = '/sys/fs/cgroup/memory/memory.limit_in_bytes'
+            if initinfo.split(' ')[0] not in ('systemd', 'init',) and os.path.exists(cgroup_limit_in_bytes):
+                with open(cgroup_limit_in_bytes) as fpc:
+                    limit_in_bytes=fpc.readline().strip()
+    return limit_in_bytes
+
 def execute_pre_post_process(d, cmds):
     if cmds is None:
         return
-- 
2.17.1

Re: [OE-core] [PATCH] bitbake.conf: update way of setting XZ_MEMLIMIT

[Richard Purdie]

On Tue, 2021-08-24 at 17:40 +0800, Changqing Li wrote:
> From: Changqing Li <changqing.li@windriver.com>
> 
> Update way of setting XZ_MEMLIMIT, considering scenario that
> running bitbake in container, to avoid OOM Killer of xz.
> 
> For example:
> Container has memory limit of 30G, and host memory is 300G.  'xz
> --memlimit=50% ...' will get host memory, so the limit for xz is 150G.
> And because of the container memory limit is 30G, xz can use nearly up
> to 30G memory, which may cause oom kill of xz.
> 
> Signed-off-by: Changqing Li <changqing.li@windriver.com>
> ---
>  meta/conf/bitbake.conf |  3 ++-
>  meta/lib/oe/utils.py   | 12 ++++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index f6fb2aa698..2b36e083ca 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -809,7 +809,8 @@ BB_NUMBER_THREADS ?= "${@oe.utils.cpu_count()}"
>  PARALLEL_MAKE ?= "-j ${@oe.utils.cpu_count()}"
>  
>  # Default parallelism and resource usage for xz
> -XZ_MEMLIMIT ?= "50%"
> +CONTAINER_MEM_LIMIT = "${@oe.utils.container_mem_limit()}"
> +XZ_MEMLIMIT ?= "${@ '%d' % (int(d.getVar('CONTAINER_MEM_LIMIT'))/2) if d.getVar('CONTAINER_MEM_LIMIT') != '0' else '50%'}"
>  XZ_THREADS ?= "${@oe.utils.cpu_count(at_least=2)}"
>  XZ_DEFAULTS ?= "--memlimit=${XZ_MEMLIMIT} --threads=${XZ_THREADS}"
>  XZ_DEFAULTS[vardepsexclude] += "XZ_MEMLIMIT XZ_THREADS"
> diff --git a/meta/lib/oe/utils.py b/meta/lib/oe/utils.py
> index a84039f585..581072ac0d 100644
> --- a/meta/lib/oe/utils.py
> +++ b/meta/lib/oe/utils.py
> @@ -252,6 +252,18 @@ def cpu_count(at_least=1):
>      cpus = len(os.sched_getaffinity(0))
>      return max(cpus, at_least)
>  
> +def container_mem_limit():
> +    limit_in_bytes = '0'
> +    proc_sched = '/proc/1/sched'
> +    if os.path.exists(proc_sched):
> +        with open(proc_sched, 'r') as fp:
> +            initinfo = fp.readline().strip()
> +            cgroup_limit_in_bytes = '/sys/fs/cgroup/memory/memory.limit_in_bytes'
> +            if initinfo.split(' ')[0] not in ('systemd', 'init',) and os.path.exists(cgroup_limit_in_bytes):
> +                with open(cgroup_limit_in_bytes) as fpc:
> +                    limit_in_bytes=fpc.readline().strip()
> +    return limit_in_bytes
> +

Whilst I understand the idea here, this is an awful lot of hardcoded values and
"magic". I'm not sure we want all bitbake processes to be making these kinds of
queries/decisions every time the config is parsed...

Adding the function to oe/utils may be ok but I'm not keen on making it the
default.

Cheers,

Richard

Re: [OE-core] [PATCH] bitbake.conf: update way of setting XZ_MEMLIMIT

[Andre McCurdy]

On Tue, Aug 24, 2021 at 7:15 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Tue, 2021-08-24 at 17:40 +0800, Changqing Li wrote:
> > From: Changqing Li <changqing.li@windriver.com>
> >
> > Update way of setting XZ_MEMLIMIT, considering scenario that
> > running bitbake in container, to avoid OOM Killer of xz.
> >
> > For example:
> > Container has memory limit of 30G, and host memory is 300G.  'xz
> > --memlimit=50% ...' will get host memory, so the limit for xz is 150G.
> > And because of the container memory limit is 30G, xz can use nearly up
> > to 30G memory, which may cause oom kill of xz.
> >
> > Signed-off-by: Changqing Li <changqing.li@windriver.com>
> > ---
> >  meta/conf/bitbake.conf |  3 ++-
> >  meta/lib/oe/utils.py   | 12 ++++++++++++
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > index f6fb2aa698..2b36e083ca 100644
> > --- a/meta/conf/bitbake.conf
> > +++ b/meta/conf/bitbake.conf
> > @@ -809,7 +809,8 @@ BB_NUMBER_THREADS ?= "${@oe.utils.cpu_count()}"
> >  PARALLEL_MAKE ?= "-j ${@oe.utils.cpu_count()}"
> >
> >  # Default parallelism and resource usage for xz
> > -XZ_MEMLIMIT ?= "50%"
> > +CONTAINER_MEM_LIMIT = "${@oe.utils.container_mem_limit()}"
> > +XZ_MEMLIMIT ?= "${@ '%d' % (int(d.getVar('CONTAINER_MEM_LIMIT'))/2) if d.getVar('CONTAINER_MEM_LIMIT') != '0' else '50%'}"
> >  XZ_THREADS ?= "${@oe.utils.cpu_count(at_least=2)}"
> >  XZ_DEFAULTS ?= "--memlimit=${XZ_MEMLIMIT} --threads=${XZ_THREADS}"
> >  XZ_DEFAULTS[vardepsexclude] += "XZ_MEMLIMIT XZ_THREADS"
> > diff --git a/meta/lib/oe/utils.py b/meta/lib/oe/utils.py
> > index a84039f585..581072ac0d 100644
> > --- a/meta/lib/oe/utils.py
> > +++ b/meta/lib/oe/utils.py
> > @@ -252,6 +252,18 @@ def cpu_count(at_least=1):
> >      cpus = len(os.sched_getaffinity(0))
> >      return max(cpus, at_least)
> >
> > +def container_mem_limit():
> > +    limit_in_bytes = '0'
> > +    proc_sched = '/proc/1/sched'
> > +    if os.path.exists(proc_sched):
> > +        with open(proc_sched, 'r') as fp:
> > +            initinfo = fp.readline().strip()
> > +            cgroup_limit_in_bytes = '/sys/fs/cgroup/memory/memory.limit_in_bytes'
> > +            if initinfo.split(' ')[0] not in ('systemd', 'init',) and os.path.exists(cgroup_limit_in_bytes):
> > +                with open(cgroup_limit_in_bytes) as fpc:
> > +                    limit_in_bytes=fpc.readline().strip()
> > +    return limit_in_bytes
> > +
>
> Whilst I understand the idea here, this is an awful lot of hardcoded values and
> "magic". I'm not sure we want all bitbake processes to be making these kinds of
> queries/decisions every time the config is parsed...

I'm sure this was discussed before but I forget the answer... how much
compression / performance is lost by setting the XZ memory limit to a
fixed value which is reasonable for all systems (e.g. 1GB? 256MB *
XZ_THREADS?).

As well as making the code simpler should also make builds more
reproducible by not depending on some detail which is specific to the
host (ie total memory size).

> Adding the function to oe/utils may be ok but I'm not keen on making it the
> default.
>
> Cheers,
>
> Richard
>
>
>
>
> 
>