[PATCH] rpm: handle virtual memory usage when limit is set

[PATCH] rpm: handle virtual memory usage when limit is set

[Peter Bergin]

Fix the situation where the task do_package_write_rpm ends up in
"liblzma: memory allocation failed". This happens if the host
environment has set a limit on virtual_memory for the user with
'ulimit -v' for packages with a lot of binary packages, e.g. glibc-locale.

Upstream-Status: Inappropriate [error introduced by oe-core patch on rpm]

Signed-off-by: Peter Bergin <peter@berginkonsult.se>
---
 ...estrict-virtual-memory-usage-if-limit-set.patch | 52 ++++++++++++++++++++++
 meta/recipes-devtools/rpm/rpm_4.14.2.bb            |  1 +
 2 files changed, 53 insertions(+)
 create mode 100644 meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch

diff --git a/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
new file mode 100644
index 0000000..a4b9a58
--- /dev/null
+++ b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
@@ -0,0 +1,52 @@
+From cff87a68cde44d893b72caa1995ac6b9a1784523 Mon Sep 17 00:00:00 2001
+From: Peter Bergin <peter@berginkonsult.se>
+Date: Wed, 19 Sep 2018 15:12:31 +0200
+Subject: [PATCH] rpm: restrict virtual memory usage if limit set
+
+A solution to avoid OOM situation when the virtual memory is restricted
+for a user (ulimit -v). As the lzopen_internal funtion is run in parallel
+one instance per CPU thread the available virtual memory is limited per
+CPU thread.
+---
+ rpmio/rpmio.c | 25 +++++++++++++++++++++++++
+ 1 file changed, 25 insertions(+)
+
+diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
+index e051c98..49752b3 100644
+--- a/rpmio/rpmio.c
++++ b/rpmio/rpmio.c
+@@ -845,6 +845,31 @@ static LZFILE *lzopen_internal(const char *mode, int fd, int xz)
+ 		}
+ #endif
+ 
++		struct rlimit virtual_memory;
++		getrlimit(RLIMIT_AS, &virtual_memory);
++		if (virtual_memory.rlim_cur != RLIM_INFINITY) {
++			const uint64_t virtual_memlimit = virtual_memory.rlim_cur;
++			const uint64_t virtual_memlimit_per_cpu_thread =
++				virtual_memlimit / lzma_cputhreads();
++			uint64_t memory_usage_virt;
++			rpmlog(RPMLOG_NOTICE, "XZ: virtual memory restricted to %lu and "
++			       "per CPU thread %lu\n", virtual_memlimit, virtual_memlimit_per_cpu_thread);
++			/* keep reducing the number of compression threads untill memory 
++			   usage gets below limit per CPU thread*/
++			while ((memory_usage_virt = lzma_stream_encoder_mt_memusage(&mt_options)) >
++			       virtual_memlimit_per_cpu_thread) {
++				/* number of threads shouldn't be able to hit zero with compression
++				 * settings aailable to set through rpm... */
++				assert(--mt_options.threads != 0);
++			}
++			if (threads != (int)mt_options.threads)
++				rpmlog(RPMLOG_NOTICE,
++				       "XZ: Adjusted the number of threads from %d to %d to not "
++				       "exceed the memory usage limit of %lu bytes\n",
++				       threads, mt_options.threads, virtual_memlimit);
++
++		}
++
+ 		ret = lzma_stream_encoder_mt(&lzfile->strm, &mt_options);
+ 	    }
+ #endif
+-- 
+2.7.4
+
diff --git a/meta/recipes-devtools/rpm/rpm_4.14.2.bb b/meta/recipes-devtools/rpm/rpm_4.14.2.bb
index 46f8837..112b41a 100644
--- a/meta/recipes-devtools/rpm/rpm_4.14.2.bb
+++ b/meta/recipes-devtools/rpm/rpm_4.14.2.bb
@@ -39,6 +39,7 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \
            file://0003-rpmstrpool.c-make-operations-over-string-pools-threa.patch \
            file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \
            file://0001-perl-disable-auto-reqs.patch \
+           file://0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch \
            "
 
 PE = "1"
-- 
2.7.4

✗ patchtest: failure for rpm: handle virtual memory usage when limit is set

[Patchwork]

== Series Details ==

Series: rpm: handle virtual memory usage when limit is set
Revision: 1
URL   : https://patchwork.openembedded.org/series/14150/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             A patch file has been added, but does not have a Signed-off-by tag [test_signed_off_by_presence] 
  Suggested fix    Sign off the added patch file (meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch)

* Issue             Added patch file is missing Upstream-Status in the header [test_upstream_status_presence_format] 
  Suggested fix    Add Upstream-Status: <Valid status> to the header of meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
  Standard format  Upstream-Status: <Valid status>
  Valid status     Pending, Accepted, Backport, Denied, Inappropriate [reason], Submitted [where]



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH] rpm: handle virtual memory usage when limit is set

[Alexander Kanavin]

2018-09-19 23:05 GMT+02:00 Peter Bergin <peter@berginkonsult.se>:
> Fix the situation where the task do_package_write_rpm ends up in
> "liblzma: memory allocation failed". This happens if the host
> environment has set a limit on virtual_memory for the user with
> 'ulimit -v' for packages with a lot of binary packages, e.g. glibc-locale.
>
> Upstream-Status: Inappropriate [error introduced by oe-core patch on rpm]

The multi-threading patches have been submitted upstream, so the
correct status here would be "Pending [merge of multithreading patches
to upstream]".

Alex

Re: [PATCH] rpm: handle virtual memory usage when limit is set

[Andre McCurdy]

On Wed, Sep 19, 2018 at 2:05 PM, Peter Bergin <peter@berginkonsult.se> wrote:
> Fix the situation where the task do_package_write_rpm ends up in
> "liblzma: memory allocation failed". This happens if the host
> environment has set a limit on virtual_memory for the user with
> 'ulimit -v' for packages with a lot of binary packages, e.g. glibc-locale.
>
> Upstream-Status: Inappropriate [error introduced by oe-core patch on rpm]
>
> Signed-off-by: Peter Bergin <peter@berginkonsult.se>
> ---
>  ...estrict-virtual-memory-usage-if-limit-set.patch | 52 ++++++++++++++++++++++
>  meta/recipes-devtools/rpm/rpm_4.14.2.bb            |  1 +
>  2 files changed, 53 insertions(+)
>  create mode 100644 meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
>
> diff --git a/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
> new file mode 100644
> index 0000000..a4b9a58
> --- /dev/null
> +++ b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
> @@ -0,0 +1,52 @@
> +From cff87a68cde44d893b72caa1995ac6b9a1784523 Mon Sep 17 00:00:00 2001
> +From: Peter Bergin <peter@berginkonsult.se>
> +Date: Wed, 19 Sep 2018 15:12:31 +0200
> +Subject: [PATCH] rpm: restrict virtual memory usage if limit set
> +
> +A solution to avoid OOM situation when the virtual memory is restricted
> +for a user (ulimit -v). As the lzopen_internal funtion is run in parallel
> +one instance per CPU thread the available virtual memory is limited per
> +CPU thread.
> +---
> + rpmio/rpmio.c | 25 +++++++++++++++++++++++++
> + 1 file changed, 25 insertions(+)
> +
> +diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
> +index e051c98..49752b3 100644
> +--- a/rpmio/rpmio.c
> ++++ b/rpmio/rpmio.c
> +@@ -845,6 +845,31 @@ static LZFILE *lzopen_internal(const char *mode, int fd, int xz)
> +               }
> + #endif
> +
> ++              struct rlimit virtual_memory;
> ++              getrlimit(RLIMIT_AS, &virtual_memory);
> ++              if (virtual_memory.rlim_cur != RLIM_INFINITY) {
> ++                      const uint64_t virtual_memlimit = virtual_memory.rlim_cur;
> ++                      const uint64_t virtual_memlimit_per_cpu_thread =

What's the point of const for these variables?

> ++                              virtual_memlimit / lzma_cputhreads();
> ++                      uint64_t memory_usage_virt;
> ++                      rpmlog(RPMLOG_NOTICE, "XZ: virtual memory restricted to %lu and "
> ++                             "per CPU thread %lu\n", virtual_memlimit, virtual_memlimit_per_cpu_thread);
> ++                      /* keep reducing the number of compression threads untill memory

Typo.

> ++                         usage gets below limit per CPU thread*/
> ++                      while ((memory_usage_virt = lzma_stream_encoder_mt_memusage(&mt_options)) >
> ++                             virtual_memlimit_per_cpu_thread) {
> ++                              /* number of threads shouldn't be able to hit zero with compression
> ++                               * settings aailable to set through rpm... */
> ++                              assert(--mt_options.threads != 0);

Putting an expression with a side effect inside assert() looks
dubious. Did you test with and without NDEBUG defined?

> ++                      }
> ++                      if (threads != (int)mt_options.threads)
> ++                              rpmlog(RPMLOG_NOTICE,
> ++                                     "XZ: Adjusted the number of threads from %d to %d to not "
> ++                                     "exceed the memory usage limit of %lu bytes\n",
> ++                                     threads, mt_options.threads, virtual_memlimit);
> ++
> ++              }
> ++
> +               ret = lzma_stream_encoder_mt(&lzfile->strm, &mt_options);
> +           }
> + #endif
> +--
> +2.7.4
> +
> diff --git a/meta/recipes-devtools/rpm/rpm_4.14.2.bb b/meta/recipes-devtools/rpm/rpm_4.14.2.bb
> index 46f8837..112b41a 100644
> --- a/meta/recipes-devtools/rpm/rpm_4.14.2.bb
> +++ b/meta/recipes-devtools/rpm/rpm_4.14.2.bb
> @@ -39,6 +39,7 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \
>             file://0003-rpmstrpool.c-make-operations-over-string-pools-threa.patch \
>             file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \
>             file://0001-perl-disable-auto-reqs.patch \
> +           file://0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch \
>             "
>
>  PE = "1"
> --
> 2.7.4
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] rpm: handle virtual memory usage when limit is set

[Peter Bergin]

On 2018-09-20 01:02, Andre McCurdy wrote:
> On Wed, Sep 19, 2018 at 2:05 PM, Peter Bergin <peter@berginkonsult.se> wrote:
>> Fix the situation where the task do_package_write_rpm ends up in
>> "liblzma: memory allocation failed". This happens if the host
>> environment has set a limit on virtual_memory for the user with
>> 'ulimit -v' for packages with a lot of binary packages, e.g. glibc-locale.
>>
>> Upstream-Status: Inappropriate [error introduced by oe-core patch on rpm]
>>
>> Signed-off-by: Peter Bergin <peter@berginkonsult.se>
>> ---
>>   ...estrict-virtual-memory-usage-if-limit-set.patch | 52 ++++++++++++++++++++++
>>   meta/recipes-devtools/rpm/rpm_4.14.2.bb            |  1 +
>>   2 files changed, 53 insertions(+)
>>   create mode 100644 meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
>>
>> diff --git a/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
>> new file mode 100644
>> index 0000000..a4b9a58
>> --- /dev/null
>> +++ b/meta/recipes-devtools/rpm/files/0001-rpm-restrict-virtual-memory-usage-if-limit-set.patch
>> @@ -0,0 +1,52 @@
>> +From cff87a68cde44d893b72caa1995ac6b9a1784523 Mon Sep 17 00:00:00 2001
>> +From: Peter Bergin <peter@berginkonsult.se>
>> +Date: Wed, 19 Sep 2018 15:12:31 +0200
>> +Subject: [PATCH] rpm: restrict virtual memory usage if limit set
>> +
>> +A solution to avoid OOM situation when the virtual memory is restricted
>> +for a user (ulimit -v). As the lzopen_internal funtion is run in parallel
>> +one instance per CPU thread the available virtual memory is limited per
>> +CPU thread.
>> +---
>> + rpmio/rpmio.c | 25 +++++++++++++++++++++++++
>> + 1 file changed, 25 insertions(+)
>> +
>> +diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
>> +index e051c98..49752b3 100644
>> +--- a/rpmio/rpmio.c
>> ++++ b/rpmio/rpmio.c
>> +@@ -845,6 +845,31 @@ static LZFILE *lzopen_internal(const char *mode, int fd, int xz)
>> +               }
>> + #endif
>> +
>> ++              struct rlimit virtual_memory;
>> ++              getrlimit(RLIMIT_AS, &virtual_memory);
>> ++              if (virtual_memory.rlim_cur != RLIM_INFINITY) {
>> ++                      const uint64_t virtual_memlimit = virtual_memory.rlim_cur;
>> ++                      const uint64_t virtual_memlimit_per_cpu_thread =
> What's the point of const for these variables?
Potential optimization. Set once and never changed will give the 
compiler a chance to optimize if possible.

>> ++                              virtual_memlimit / lzma_cputhreads();
>> ++                      uint64_t memory_usage_virt;
>> ++                      rpmlog(RPMLOG_NOTICE, "XZ: virtual memory restricted to %lu and "
>> ++                             "per CPU thread %lu\n", virtual_memlimit, virtual_memlimit_per_cpu_thread);
>> ++                      /* keep reducing the number of compression threads untill memory
> Typo.
>
>> ++                         usage gets below limit per CPU thread*/
>> ++                      while ((memory_usage_virt = lzma_stream_encoder_mt_memusage(&mt_options)) >
>> ++                             virtual_memlimit_per_cpu_thread) {
>> ++                              /* number of threads shouldn't be able to hit zero with compression
>> ++                               * settings aailable to set through rpm... */
>> ++                              assert(--mt_options.threads != 0);
> Putting an expression with a side effect inside assert() looks
> dubious. Did you test with and without NDEBUG defined?
No. Just copy-paste from code in the same file and same function.

Regards,
/Peter