From b8371d532d6cd1314954c2d3e9b8a79d3d70e7c7 Mon Sep 17 00:00:00 2001 From: Hridya Valsaraju Date: Thu, 31 May 2018 12:39:58 -0700 Subject: [PATCH 01/10] Populate recovery DTBO offset correctly Also, remove recovery_dtbo_offset argument for mkbootimg as this is calculated based on sizes of kernel, ramdisk and second binaries. Also, modify unpack_bootimg to use the recovery_dtbo_offset field to extract recovery_dtbo. Test: make recoveryimage showcommands -j32 Bug: 80207223 unpack_bootimg --boot_img $OUT/recovery.img diff recovery_dtbo dtbo.img Change-Id: I588ccc8b739c169b6f78c17ffe554c5562397d98 Merged-In: I588ccc8b739c169b6f78c17ffe554c5562397d98 (cherry picked from commit 26e01bbdc5cb2dcad64d73ec836447b8ded4fc6c) --- mkbootimg/mkbootimg | 27 ++++++++++++++++++++++----- mkbootimg/unpack_bootimg | 9 +++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/mkbootimg/mkbootimg b/mkbootimg/mkbootimg index ac20d0503..fda9af0d2 100755 --- a/mkbootimg/mkbootimg +++ b/mkbootimg/mkbootimg @@ -45,6 +45,22 @@ def pad_file(f, padding): f.write(pack(str(pad) + 'x')) +def get_number_of_pages(image_size, page_size): + """calculates the number of pages required for the image""" + return (image_size + page_size - 1) / page_size + + +def get_recovery_dtbo_offset(args): + """calculates the offset of recovery_dtbo image in the boot image""" + num_header_pages = 1 # header occupies a page + num_kernel_pages = get_number_of_pages(filesize(args.kernel), args.pagesize) + num_ramdisk_pages = get_number_of_pages(filesize(args.ramdisk), args.pagesize) + num_second_pages = get_number_of_pages(filesize(args.second), args.pagesize) + dtbo_offset = args.pagesize * (num_header_pages + num_kernel_pages + + num_ramdisk_pages + num_second_pages) + return dtbo_offset + + def write_header(args): BOOT_MAGIC = 'ANDROID!'.encode() args.output.write(pack('8s', BOOT_MAGIC)) @@ -76,9 +92,12 @@ def write_header(args): args.output.write(pack('1024s', args.cmdline[512:].encode())) if args.header_version > 0: - args.output.write(pack('I', filesize(args.recovery_dtbo))) # size in bytes - args.output.write(pack('Q', args.base + args.recovery_dtbo_offset)) # physical load addr - args.output.write(pack('I', args.output.tell() + 4)) # size of boot header + args.output.write(pack('I', filesize(args.recovery_dtbo))) # size in bytes + if args.recovery_dtbo: + args.output.write(pack('Q', get_recovery_dtbo_offset(args))) # recovery dtbo offset + else: + args.output.write(pack('Q', 0)) # Will be set to 0 for devices without a recovery dtbo + args.output.write(pack('I', args.output.tell() + 4)) # size of boot header pad_file(args.output, args.pagesize) return img_id @@ -150,8 +169,6 @@ def parse_cmdline(): parser.add_argument('--ramdisk_offset', help='ramdisk offset', type=parse_int, default=0x01000000) parser.add_argument('--second_offset', help='2nd bootloader offset', type=parse_int, default=0x00f00000) - parser.add_argument('--recovery_dtbo_offset', help='recovery dtbo offset', type=parse_int, - default=0x0f000000) parser.add_argument('--os_version', help='operating system version', type=parse_os_version, default=0) parser.add_argument('--os_patch_level', help='operating system patch level', diff --git a/mkbootimg/unpack_bootimg b/mkbootimg/unpack_bootimg index 8e42ec029..c37acd5ac 100755 --- a/mkbootimg/unpack_bootimg +++ b/mkbootimg/unpack_bootimg @@ -76,8 +76,8 @@ def unpack_bootimage(args): if version > 0: recovery_dtbo_size = unpack('I', args.boot_img.read(1 * 4))[0] print('recovery dtbo size: %s' % recovery_dtbo_size) - recovery_dtbo_address = unpack('Q', args.boot_img.read(8))[0] - print('recovery dtbo load address: %s' % recovery_dtbo_address) + recovery_dtbo_offset = unpack('Q', args.boot_img.read(8))[0] + print('recovery dtbo offset: %s' % recovery_dtbo_offset) boot_header_size = unpack('I', args.boot_img.read(4))[0] print('boot header size: %s' % boot_header_size) else: @@ -95,16 +95,13 @@ def unpack_bootimage(args): ) # header + kernel image_info_list.append((ramdisk_offset, ramdisk_size, 'ramdisk')) - num_second_pages = get_number_of_pages(second_size, page_size) second_offset = page_size * ( num_header_pages + num_kernel_pages + num_ramdisk_pages ) # header + kernel + ramdisk image_info_list.append((second_offset, second_size, 'second')) if recovery_dtbo_size > 0: - dtbo_offset = page_size * (num_header_pages + num_kernel_pages + - num_ramdisk_pages + num_second_pages) - image_info_list.append((dtbo_offset, recovery_dtbo_size, + image_info_list.append((recovery_dtbo_offset, recovery_dtbo_size, 'recovery_dtbo')) for image_info in image_info_list: From f514f6f66aba10f2668481a4d643c7983a0ea93a Mon Sep 17 00:00:00 2001 From: Hridya Valsaraju Date: Wed, 30 May 2018 10:57:18 -0700 Subject: [PATCH 02/10] Correct comment in boot image header Test: make Bug: 80207223 Change-Id: I88cf7683e036ff43bbe3ff5418519c7e35e08b79 Merged-In: I88cf7683e036ff43bbe3ff5418519c7e35e08b79 (cherry picked from commit d0ab4e3701dcec866cb05f4d2a122fbbcabf61cc) --- mkbootimg/include/bootimg/bootimg.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mkbootimg/include/bootimg/bootimg.h b/mkbootimg/include/bootimg/bootimg.h index 4311b46f9..0188d0e94 100644 --- a/mkbootimg/include/bootimg/bootimg.h +++ b/mkbootimg/include/bootimg/bootimg.h @@ -107,7 +107,7 @@ typedef struct boot_img_hdr_v0 boot_img_hdr; struct boot_img_hdr_v1 : public boot_img_hdr_v0 { uint32_t recovery_dtbo_size; /* size in bytes for recovery DTBO image */ - uint64_t recovery_dtbo_offset; /* physical load addr */ + uint64_t recovery_dtbo_offset; /* offset to recovery dtbo in boot image */ uint32_t header_size; } __attribute__((packed)); @@ -134,12 +134,15 @@ struct boot_img_hdr_v1 : public boot_img_hdr_v0 { * 1. kernel and ramdisk are required (size != 0) * 2. recovery_dtbo is required for recovery.img in non-A/B devices(recovery_dtbo_size != 0) * 3. second is optional (second_size == 0 -> no second) - * 4. load each element (kernel, ramdisk, second, recovery_dtbo) at + * 4. load each element (kernel, ramdisk, second) at * the specified physical address (kernel_addr, etc) - * 5. prepare tags at tag_addr. kernel_args[] is + * 5. If booting to recovery mode in a non-A/B device, extract recovery dtbo and + * apply the correct set of overlays on the base device tree depending on the + * hardware/product revision. + * 6. prepare tags at tag_addr. kernel_args[] is * appended to the kernel commandline in the tags. - * 6. r0 = 0, r1 = MACHINE_TYPE, r2 = tags_addr - * 7. if second_size != 0: jump to second_addr + * 7. r0 = 0, r1 = MACHINE_TYPE, r2 = tags_addr + * 8. if second_size != 0: jump to second_addr * else: jump to kernel_addr */ From 299276785787a35effa829f3f9d507cbde266dea Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Thu, 14 Jun 2018 15:12:26 -0700 Subject: [PATCH 03/10] healthd: add stem to health@2.0-service.override [DO NOT MERGE] The override module should have the same name as the -service module; the only difference is that healthd is installed or not. Without this change, the binary is completely useless due to incorrect rc files and sepolicy file_contexts. This allows emulator to use this module correctly. Bug: 110228707 Test: builds on emulator, boots Change-Id: Id2062925ec3567c4fde70358184bb790541bf83b --- healthd/Android.bp | 1 + 1 file changed, 1 insertion(+) diff --git a/healthd/Android.bp b/healthd/Android.bp index cefe09d5e..0f6e8372c 100644 --- a/healthd/Android.bp +++ b/healthd/Android.bp @@ -61,6 +61,7 @@ cc_binary { cc_binary { name: "android.hardware.health@2.0-service.override", + stem: "android.hardware.health@2.0-service", defaults: ["android.hardware.health@2.0-service_defaults"], overrides: [ From 9a95028eac3bb01a4c04012a62a4588d9eed5558 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Mon, 18 Jun 2018 21:22:49 +0000 Subject: [PATCH 04/10] Revert "healthd: add stem to health@2.0-service.override [DO NOT MERGE]" This should have been done with actual fixes to sepolicy and rc scripts. This reverts commit 299276785787a35effa829f3f9d507cbde266dea. Reason for revert: broke the build Bug: 110228707 Change-Id: I23944d044dd2f87e6c1033ed7a4c7cf34f16247e --- healthd/Android.bp | 1 - 1 file changed, 1 deletion(-) diff --git a/healthd/Android.bp b/healthd/Android.bp index 0f6e8372c..cefe09d5e 100644 --- a/healthd/Android.bp +++ b/healthd/Android.bp @@ -61,7 +61,6 @@ cc_binary { cc_binary { name: "android.hardware.health@2.0-service.override", - stem: "android.hardware.health@2.0-service", defaults: ["android.hardware.health@2.0-service_defaults"], overrides: [ From c4cf62f4b4e9186a897e943b2419fb9a13739c1b Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Mon, 18 Jun 2018 13:21:45 +0900 Subject: [PATCH 05/10] Add /system/product/* paths to permitted paths As linker doesn't resolve paths in permitted paths, /system/product variants should be added to support devices having product partition under /system. Bug: 110286945 Test: m -j succeeds on taimen and libraries under /system can dlopen libraries under /system/product/apps Change-Id: Icd102d44511702e4ec66c07a367b59c3d9700a44 Merged-In: Icd102d44511702e4ec66c07a367b59c3d9700a44 (cherry picked from commit 3918936b9e7fee51828d285a1217c7202d5cb2d1) --- rootdir/etc/ld.config.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rootdir/etc/ld.config.txt b/rootdir/etc/ld.config.txt index a0b1996ad..eebad2b3c 100644 --- a/rootdir/etc/ld.config.txt +++ b/rootdir/etc/ld.config.txt @@ -52,6 +52,7 @@ namespace.default.permitted.paths = /system/${LIB}/drm namespace.default.permitted.paths += /system/${LIB}/extractors namespace.default.permitted.paths += /system/${LIB}/hw namespace.default.permitted.paths += /product/${LIB} +namespace.default.permitted.paths += /system/product/${LIB} # These are where odex files are located. libart has to be able to dlopen the files namespace.default.permitted.paths += /system/framework namespace.default.permitted.paths += /system/app @@ -66,6 +67,9 @@ namespace.default.permitted.paths += /oem/app namespace.default.permitted.paths += /product/framework namespace.default.permitted.paths += /product/app namespace.default.permitted.paths += /product/priv-app +namespace.default.permitted.paths += /system/product/framework +namespace.default.permitted.paths += /system/product/app +namespace.default.permitted.paths += /system/product/priv-app namespace.default.permitted.paths += /data namespace.default.permitted.paths += /mnt/expand @@ -92,6 +96,10 @@ namespace.default.asan.permitted.paths += /product/${LIB} namespace.default.asan.permitted.paths += /product/framework namespace.default.asan.permitted.paths += /product/app namespace.default.asan.permitted.paths += /product/priv-app +namespace.default.asan.permitted.paths += /system/product/${LIB} +namespace.default.asan.permitted.paths += /system/product/framework +namespace.default.asan.permitted.paths += /system/product/app +namespace.default.asan.permitted.paths += /system/product/priv-app namespace.default.asan.permitted.paths += /mnt/expand ############################################################################### From 93d837f3a90acec007647f21ed4573f044fa6f1e Mon Sep 17 00:00:00 2001 From: Doheon Lee Date: Tue, 19 Jun 2018 15:23:21 +0900 Subject: [PATCH 06/10] Determine product partition path on build time Path of product partitoin can be set as /product or /system/product whether generate extra product partition or not. Substitute %PRODUCT% to relevant path to know linker which path should search and permit. Bug: 110286945 Test: m -j # Check /system/etc/ld.config.$(PLATFORM_VNDK_VERSION).txt Change-Id: I6ca177d0c9c5af00ad821879fece40848331fc8d Merged-In: I6ca177d0c9c5af00ad821879fece40848331fc8d (cherry picked from commit cccad0bf8461ab575a5861f9bc68c9f8f69db582) --- rootdir/Android.mk | 1 + rootdir/etc/ld.config.txt | 32 ++++++++++++-------------------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/rootdir/Android.mk b/rootdir/Android.mk index 80e068af7..5f5c363a7 100644 --- a/rootdir/Android.mk +++ b/rootdir/Android.mk @@ -204,6 +204,7 @@ $(2): $(1) $$(hide) sed -i -e 's?%VNDK_CORE_LIBRARIES%?$$(PRIVATE_VNDK_CORE_LIBRARIES)?g' $$@ $$(hide) sed -i -e 's?%SANITIZER_RUNTIME_LIBRARIES%?$$(PRIVATE_SANITIZER_RUNTIME_LIBRARIES)?g' $$@ $$(hide) sed -i -e 's?%VNDK_VER%?$$(PRIVATE_VNDK_VERSION)?g' $$@ + $$(hide) sed -i -e 's?%PRODUCT%?$$(TARGET_COPY_OUT_PRODUCT)?g' $$@ llndk_libraries_list := vndksp_libraries_list := diff --git a/rootdir/etc/ld.config.txt b/rootdir/etc/ld.config.txt index eebad2b3c..42dc7abe7 100644 --- a/rootdir/etc/ld.config.txt +++ b/rootdir/etc/ld.config.txt @@ -7,7 +7,7 @@ # absolute path of an executable is selected. dir.system = /system/bin/ dir.system = /system/xbin/ -dir.system = /product/bin/ +dir.system = /%PRODUCT%/bin/ dir.vendor = /odm/bin/ dir.vendor = /vendor/bin/ @@ -39,7 +39,7 @@ additional.namespaces = sphal,vndk,rs namespace.default.isolated = true namespace.default.search.paths = /system/${LIB} -namespace.default.search.paths += /product/${LIB} +namespace.default.search.paths += /%PRODUCT%/${LIB} # We can't have entire /system/${LIB} as permitted paths because doing so # makes it possible to load libs in /system/${LIB}/vndk* directories by @@ -51,8 +51,7 @@ namespace.default.search.paths += /product/${LIB} namespace.default.permitted.paths = /system/${LIB}/drm namespace.default.permitted.paths += /system/${LIB}/extractors namespace.default.permitted.paths += /system/${LIB}/hw -namespace.default.permitted.paths += /product/${LIB} -namespace.default.permitted.paths += /system/product/${LIB} +namespace.default.permitted.paths += /%PRODUCT%/${LIB} # These are where odex files are located. libart has to be able to dlopen the files namespace.default.permitted.paths += /system/framework namespace.default.permitted.paths += /system/app @@ -64,12 +63,9 @@ namespace.default.permitted.paths += /odm/framework namespace.default.permitted.paths += /odm/app namespace.default.permitted.paths += /odm/priv-app namespace.default.permitted.paths += /oem/app -namespace.default.permitted.paths += /product/framework -namespace.default.permitted.paths += /product/app -namespace.default.permitted.paths += /product/priv-app -namespace.default.permitted.paths += /system/product/framework -namespace.default.permitted.paths += /system/product/app -namespace.default.permitted.paths += /system/product/priv-app +namespace.default.permitted.paths += /%PRODUCT%/framework +namespace.default.permitted.paths += /%PRODUCT%/app +namespace.default.permitted.paths += /%PRODUCT%/priv-app namespace.default.permitted.paths += /data namespace.default.permitted.paths += /mnt/expand @@ -92,14 +88,10 @@ namespace.default.asan.permitted.paths += /odm/framework namespace.default.asan.permitted.paths += /odm/app namespace.default.asan.permitted.paths += /odm/priv-app namespace.default.asan.permitted.paths += /oem/app -namespace.default.asan.permitted.paths += /product/${LIB} -namespace.default.asan.permitted.paths += /product/framework -namespace.default.asan.permitted.paths += /product/app -namespace.default.asan.permitted.paths += /product/priv-app -namespace.default.asan.permitted.paths += /system/product/${LIB} -namespace.default.asan.permitted.paths += /system/product/framework -namespace.default.asan.permitted.paths += /system/product/app -namespace.default.asan.permitted.paths += /system/product/priv-app +namespace.default.asan.permitted.paths += /%PRODUCT%/${LIB} +namespace.default.asan.permitted.paths += /%PRODUCT%/framework +namespace.default.asan.permitted.paths += /%PRODUCT%/app +namespace.default.asan.permitted.paths += /%PRODUCT%/priv-app namespace.default.asan.permitted.paths += /mnt/expand ############################################################################### @@ -335,7 +327,7 @@ namespace.vndk.link.default.allow_all_shared_libs = true namespace.system.isolated = false namespace.system.search.paths = /system/${LIB} -namespace.system.search.paths += /product/${LIB} +namespace.system.search.paths += /%PRODUCT%/${LIB} namespace.system.asan.search.paths = /data/asan/system/${LIB} namespace.system.asan.search.paths += /system/${LIB} @@ -353,4 +345,4 @@ namespace.system.asan.search.paths += /product/${LIB} [postinstall] namespace.default.isolated = false namespace.default.search.paths = /system/${LIB} -namespace.default.search.paths += /product/${LIB} +namespace.default.search.paths += /%PRODUCT%/${LIB} From ae2e19dd96e47a084eac09c3b3095b3bef427415 Mon Sep 17 00:00:00 2001 From: Thierry Strudel Date: Thu, 19 Jul 2018 18:49:15 -0700 Subject: [PATCH 07/10] metricslogger: fix ACTION_BATTERY_CAUSED_SHUTDOWN not matching proto Bug: 111664962 Test: "pixelstats_client -s" uses the right ATOM value Change-Id: I2944c6d9f79298ce88812fad218552b92afee97a Signed-off-by: Thierry Strudel --- libmetricslogger/include/metricslogger/metrics_logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmetricslogger/include/metricslogger/metrics_logger.h b/libmetricslogger/include/metricslogger/metrics_logger.h index c305db2da..88575be9d 100644 --- a/libmetricslogger/include/metricslogger/metrics_logger.h +++ b/libmetricslogger/include/metricslogger/metrics_logger.h @@ -114,7 +114,7 @@ enum { FIELD_BATTERY_RESISTANCE_UOHMS = 1448, FIELD_BATTERY_CURRENT_UA = 1449, FIELD_HARDWARE_LOCATION = 1450, - ACTION_BATTERY_CAUSED_SHUTDOWN = 1441, + ACTION_BATTERY_CAUSED_SHUTDOWN = 1451, }; enum { From 270e90f033dd412206697e3a27aee76c24038c33 Mon Sep 17 00:00:00 2001 From: Greg Kaiser Date: Wed, 1 Aug 2018 12:34:32 -0700 Subject: [PATCH 08/10] Revert "Support Speck encryption." This reverts commit 49c27c5cb2c48ce4665affe2b3086077c768a9ae. Remove the Speck encryption support. It was eventually decided not to allow Speck in Android P, so this code is no longer needed and wasn't used outside of testing. Bug: 112009351 Test: Confirmed AES continues to work with FBE. Change-Id: Ia5458143be5687fff8d541d8fa2c8ee24a369da4 --- fs_mgr/fs_mgr_fstab.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 68cc530dd..c9fb7aa57 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -114,21 +114,17 @@ static struct flag_list fs_mgr_flags[] = { #define EM_ICE 2 #define EM_AES_256_CTS 3 #define EM_AES_256_HEH 4 -#define EM_SPECK_128_256_XTS 5 -#define EM_SPECK_128_256_CTS 6 static const struct flag_list file_contents_encryption_modes[] = { {"aes-256-xts", EM_AES_256_XTS}, - {"speck128/256-xts", EM_SPECK_128_256_XTS}, {"software", EM_AES_256_XTS}, /* alias for backwards compatibility */ - {"ice", EM_ICE}, /* hardware-specific inline cryptographic engine */ + {"ice", EM_ICE}, /* hardware-specific inline cryptographic engine */ {0, 0}, }; static const struct flag_list file_names_encryption_modes[] = { {"aes-256-cts", EM_AES_256_CTS}, {"aes-256-heh", EM_AES_256_HEH}, - {"speck128/256-cts", EM_SPECK_128_256_CTS}, {0, 0}, }; From d2e1a669c6ed6c9239c4d5f52981ab1b39df12f1 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 5 Sep 2018 15:46:32 -0700 Subject: [PATCH 09/10] lmkd: rate-limit and cleanup failed kill reports Excessive number of failed kill reports when lmkd can't find an eligible process to kill or frees not enough memory pollutes logs and bugreports. Cleanup kill reports to remove duplicate information and rate limit failed kill attempts at 1 report per sec. The number of suppressed failed kills will be reported in the next lmkd report. Bug: 113864581 Test: Verified using lmkd_unit_test Change-Id: I67fa1fec97613f136c7582115edcbc56b1503c9c Merged-In: I67fa1fec97613f136c7582115edcbc56b1503c9c Signed-off-by: Suren Baghdasaryan (cherry picked from commit d6cbf3f41d7cb54e7dc62f4c29f7a7cba05aa28b) --- lmkd/lmkd.c | 80 +++++++++++++++++++++++----------------- lmkd/tests/lmkd_test.cpp | 2 +- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/lmkd/lmkd.c b/lmkd/lmkd.c index cc5a7c2cf..bf265b0f2 100644 --- a/lmkd/lmkd.c +++ b/lmkd/lmkd.c @@ -79,6 +79,8 @@ #define STRINGIFY(x) STRINGIFY_INTERNAL(x) #define STRINGIFY_INTERNAL(x) #x +#define FAIL_REPORT_RLIMIT_MS 1000 + /* default to old in-kernel interface if no memory pressure events */ static int use_inkernel_interface = 1; static bool has_inkernel_module; @@ -956,8 +958,7 @@ static struct proc *proc_get_heaviest(int oomadj) { } /* Kill one process specified by procp. Returns the size of the process killed */ -static int kill_one_process(struct proc* procp, int min_score_adj, - enum vmpressure_level level) { +static int kill_one_process(struct proc* procp) { int pid = procp->pid; uid_t uid = procp->uid; char *taskname; @@ -990,11 +991,8 @@ static int kill_one_process(struct proc* procp, int min_score_adj, TRACE_KILL_START(pid); r = kill(pid, SIGKILL); - ALOGI( - "Killing '%s' (%d), uid %d, adj %d\n" - " to free %ldkB because system is under %s memory pressure oom_adj %d\n", - taskname, pid, uid, procp->oomadj, tasksize * page_k, - level_name[level], min_score_adj); + ALOGI("Kill '%s' (%d), uid %d, oom_adj %d to free %ldkB", + taskname, pid, uid, procp->oomadj, tasksize * page_k); pid_remove(pid); TRACE_KILL_END(); @@ -1021,8 +1019,7 @@ static int kill_one_process(struct proc* procp, int min_score_adj, * If pages_to_free is set to 0 only one process will be killed. * Returns the size of the killed processes. */ -static int find_and_kill_processes(enum vmpressure_level level, - int min_score_adj, int pages_to_free) { +static int find_and_kill_processes(int min_score_adj, int pages_to_free) { int i; int killed_size; int pages_freed = 0; @@ -1041,7 +1038,7 @@ static int find_and_kill_processes(enum vmpressure_level level, if (!procp) break; - killed_size = kill_one_process(procp, min_score_adj, level); + killed_size = kill_one_process(procp); if (killed_size >= 0) { #ifdef LMKD_LOG_STATS if (enable_stats_log && !lmk_state_change_start) { @@ -1146,8 +1143,9 @@ static void mp_event_common(int data, uint32_t events __unused) { enum vmpressure_level lvl; union meminfo mi; union zoneinfo zi; - static struct timeval last_report_tm; - static unsigned long skip_count = 0; + struct timeval curr_tm; + static struct timeval last_kill_tm; + static unsigned long kill_skip_count = 0; enum vmpressure_level level = (enum vmpressure_level)data; long other_free = 0, other_file = 0; int min_score_adj; @@ -1176,19 +1174,18 @@ static void mp_event_common(int data, uint32_t events __unused) { } } + gettimeofday(&curr_tm, NULL); if (kill_timeout_ms) { - struct timeval curr_tm; - gettimeofday(&curr_tm, NULL); - if (get_time_diff_ms(&last_report_tm, &curr_tm) < kill_timeout_ms) { - skip_count++; + if (get_time_diff_ms(&last_kill_tm, &curr_tm) < kill_timeout_ms) { + kill_skip_count++; return; } } - if (skip_count > 0) { + if (kill_skip_count > 0) { ALOGI("%lu memory pressure events were skipped after a kill!", - skip_count); - skip_count = 0; + kill_skip_count); + kill_skip_count = 0; } if (meminfo_parse(&mi) < 0 || zoneinfo_parse(&zi) < 0) { @@ -1280,13 +1277,15 @@ static void mp_event_common(int data, uint32_t events __unused) { do_kill: if (low_ram_device) { /* For Go devices kill only one task */ - if (find_and_kill_processes(level, level_oomadj[level], 0) == 0) { + if (find_and_kill_processes(level_oomadj[level], 0) == 0) { if (debug_process_killing) { ALOGI("Nothing to kill"); } } } else { int pages_freed; + static struct timeval last_report_tm; + static unsigned long report_skip_count = 0; if (!use_minfree_levels) { /* If pressure level is less than critical and enough free swap then ignore */ @@ -1314,24 +1313,39 @@ do_kill: min_score_adj = level_oomadj[level]; } - pages_freed = find_and_kill_processes(level, min_score_adj, pages_to_free); + pages_freed = find_and_kill_processes(min_score_adj, pages_to_free); + + if (pages_freed == 0) { + /* Rate limit kill reports when nothing was reclaimed */ + if (get_time_diff_ms(&last_report_tm, &curr_tm) < FAIL_REPORT_RLIMIT_MS) { + report_skip_count++; + return; + } + } + + if (pages_freed >= pages_to_free) { + /* Reset kill time only if reclaimed enough memory */ + last_kill_tm = curr_tm; + } if (use_minfree_levels) { - ALOGI("Killing because cache %ldkB is below " - "limit %ldkB for oom_adj %d\n" - " Free memory is %ldkB %s reserved", - other_file * page_k, minfree * page_k, min_score_adj, - other_free * page_k, other_free >= 0 ? "above" : "below"); + ALOGI("Killing to reclaim %ldkB, reclaimed %ldkB, cache(%ldkB) and " + "free(%" PRId64 "kB)-reserved(%" PRId64 "kB) below min(%ldkB) for oom_adj %d", + pages_to_free * page_k, pages_freed * page_k, + other_file * page_k, mi.field.nr_free_pages * page_k, + zi.field.totalreserve_pages * page_k, + minfree * page_k, min_score_adj); + } else { + ALOGI("Killing to reclaim %ldkB, reclaimed %ldkB at oom_adj %d", + pages_to_free * page_k, pages_freed * page_k, min_score_adj); } - if (pages_freed < pages_to_free) { - ALOGI("Unable to free enough memory (pages to free=%d, pages freed=%d)", - pages_to_free, pages_freed); - } else { - ALOGI("Reclaimed enough memory (pages to free=%d, pages freed=%d)", - pages_to_free, pages_freed); - gettimeofday(&last_report_tm, NULL); + if (report_skip_count > 0) { + ALOGI("Suppressed %lu failed kill reports", report_skip_count); + report_skip_count = 0; } + + last_report_tm = curr_tm; } } diff --git a/lmkd/tests/lmkd_test.cpp b/lmkd/tests/lmkd_test.cpp index 4afaeb81f..c2ad74a71 100644 --- a/lmkd/tests/lmkd_test.cpp +++ b/lmkd/tests/lmkd_test.cpp @@ -39,7 +39,7 @@ using namespace android::base; #define LMKDTEST_RESPAWN_FLAG "LMKDTEST_RESPAWN" #define LMKD_LOGCAT_MARKER "lowmemorykiller" -#define LMKD_KILL_MARKER_TEMPLATE LMKD_LOGCAT_MARKER ": Killing '%s'" +#define LMKD_KILL_MARKER_TEMPLATE LMKD_LOGCAT_MARKER ": Kill '%s'" #define OOM_MARKER "Out of memory" #define OOM_KILL_MARKER "Killed process" #define MIN_LOG_SIZE 100 From 04675d7781c99cb68e41c251645b2df0ff8d81e5 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Thu, 25 Oct 2018 17:05:41 -0700 Subject: [PATCH 10/10] DO NOT MERGE: lmkd: retune rate at which processes are killed Kill a single process at a time and try to wait up to 100ms for that process to reclaim memory before triggering another kill. Test: boots, works bug: 116877958 Change-Id: I6775d0534b3e3728c04389d3eae1a00e3cbf9f27 (cherry picked from commit afb3a15f39e73978c071a58d3466993293c6d3b1) --- lmkd/lmkd.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/lmkd/lmkd.c b/lmkd/lmkd.c index bf265b0f2..6e2945376 100644 --- a/lmkd/lmkd.c +++ b/lmkd/lmkd.c @@ -27,8 +27,8 @@ #include #include #include -#include #include +#include #include #include @@ -957,6 +957,8 @@ static struct proc *proc_get_heaviest(int oomadj) { return maxprocp; } +static int last_killed_pid = -1; + /* Kill one process specified by procp. Returns the size of the process killed */ static int kill_one_process(struct proc* procp) { int pid = procp->pid; @@ -997,6 +999,8 @@ static int kill_one_process(struct proc* procp) { TRACE_KILL_END(); + last_killed_pid = pid; + if (r) { ALOGE("kill(%d): errno=%d", pid, errno); return -1; @@ -1135,6 +1139,22 @@ static inline unsigned long get_time_diff_ms(struct timeval *from, (to->tv_usec - from->tv_usec) / 1000; } +static bool is_kill_pending(void) { + char buf[24]; + if (last_killed_pid < 0) { + return false; + } + + snprintf(buf, sizeof(buf), "/proc/%d/", last_killed_pid); + if (access(buf, F_OK) == 0) { + return true; + } + + // reset last killed PID because there's nothing pending + last_killed_pid = -1; + return false; +} + static void mp_event_common(int data, uint32_t events __unused) { int ret; unsigned long long evcount; @@ -1176,7 +1196,11 @@ static void mp_event_common(int data, uint32_t events __unused) { gettimeofday(&curr_tm, NULL); if (kill_timeout_ms) { - if (get_time_diff_ms(&last_kill_tm, &curr_tm) < kill_timeout_ms) { + // If we're within the timeout, see if there's pending reclaim work + // from the last killed process. If there is (as evidenced by + // /proc/ continuing to exist), skip killing for now. + if ((get_time_diff_ms(&last_kill_tm, &curr_tm) < kill_timeout_ms) && + (low_ram_device || is_kill_pending())) { kill_skip_count++; return; } @@ -1313,7 +1337,7 @@ do_kill: min_score_adj = level_oomadj[level]; } - pages_freed = find_and_kill_processes(min_score_adj, pages_to_free); + pages_freed = find_and_kill_processes(min_score_adj, 0); if (pages_freed == 0) { /* Rate limit kill reports when nothing was reclaimed */ @@ -1321,10 +1345,8 @@ do_kill: report_skip_count++; return; } - } - - if (pages_freed >= pages_to_free) { - /* Reset kill time only if reclaimed enough memory */ + } else { + /* If we killed anything, update the last killed timestamp. */ last_kill_tm = curr_tm; }