[OE-core] [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 at 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 at 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 at 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