[PATCH RFC i-g-t] lib/igt_kmemleak: library to interact with kmemleak
Vivekanandan, Balasubramani
balasubramani.vivekanandan at intel.com
Wed Jan 8 05:23:27 UTC 2025
On 08.01.2025 01:47, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Peter Senna Tschudin
> Sent: Monday, December 16, 2024 10:47 AM
> To: igt-dev at lists.freedesktop.org
> Cc: Senna, Peter <peter.senna at intel.com>; Gandi, Ramadevi <ramadevi.gandi at intel.com>; Knop, Ryszard <ryszard.knop at intel.com>; Lattannavar, Sameer <sameer.lattannavar at intel.com>; De Marchi, Lucas <lucas.demarchi at intel.com>; Saarinen, Jani <jani.saarinen at intel.com>; Piecielska, Katarzyna <katarzyna.piecielska at intel.com>; Roper, Matthew D <matthew.d.roper at intel.com>; gregory.f.germano at intel.com; Taylor, Clinton A <clinton.a.taylor at intel.com>; Vivekanandan, Balasubramani <balasubramani.vivekanandan at intel.com>; Yu, Jianshui <jianshui.yu at intel.com>; Peter Senna Tschudin <peter.senna at linux.intel.com>
> Subject: [PATCH RFC i-g-t] lib/igt_kmemleak: library to interact with kmemleak
> >
> > From: Peter Senna Tschudin <peter.senna at intel.com>
> >
> > Adds a simple library for interacting with kmemleak ispired by
> > 'tests/amdgpu/amd_mem_leak.c'. Also adds unit testing for the
> > library.
> >
> > To use the library include "igt_kmemleak.h" and:
> > 1. igt_kmemleak_init(NULL) /* Returns true if kmemleak is active */
> > 2. igt_kmemleak_clear()
> > 3. /* Run your test */
> > 4. igt_kmemleak_scan()
> > 5. if (igt_kmemleak_found_leaks())
> > igt_kmemleak_cp_leaks_file(*dst)
> >
> > CC: ramadevi.gandi at intel.com
> > CC: ryszard.knop at intel.com
> > CC: sameer.lattannavar at intel.com
> > CC: lucas.demarchi at intel.com
> > CC: jani.saarinen at intel.com
> > CC: katarzyna.piecielska at intel.com
> > CC: matthew.d.roper at intel.com
> > CC: gregory.f.germano at intel.com
> > CC: clinton.a.taylor at intel.com
> > CC: balasubramani.vivekanandan at intel.com
> > CC: jianshui.yu at intel.com
> > Signed-off-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
>
> I'm not particularly well-versed in kmemleak, but nothing about this patch seems out of the
> ordinary. Though I can still offer some small style-related suggestions.
> Also, do you want a reviewed-by for this? I'd give one right now but I don't think I should be
> Acking RFC patches.
>
> > ---
> > lib/igt_kmemleak.c | 154 +++++++++++++++++++++
> > lib/igt_kmemleak.h | 69 ++++++++++
> > lib/meson.build | 1 +
> > lib/tests/igt_kmemleak.c | 291 +++++++++++++++++++++++++++++++++++++++
> > lib/tests/meson.build | 1 +
> > 5 files changed, 516 insertions(+)
> > create mode 100644 lib/igt_kmemleak.c
> > create mode 100644 lib/igt_kmemleak.h
> > create mode 100644 lib/tests/igt_kmemleak.c
> >
> > diff --git a/lib/igt_kmemleak.c b/lib/igt_kmemleak.c
> > new file mode 100644
> > index 000000000..02ee0361d
> > --- /dev/null
> > +++ b/lib/igt_kmemleak.c
> > @@ -0,0 +1,154 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#include <ctype.h>
> > +#include <libudev.h>
> > +#include <stdio.h>
> > +#include <sys/time.h>
> > +#include <time.h>
> > +
> > +#include "igt_core.h"
> > +#include "igt_device_scan.h"
> > +#include "igt_kmemleak.h"
> > +#include "igt_kmod.h"
> > +#include "igt_list.h"
> > +
> > +/* We can change the path for unit testing, see igt_kmemleak_init() */
> > +static char igt_kmemleak_file[256] = "/sys/kernel/debug/kmemleak";
> > +
> > +/**
> > + * igt_kmemeak_init:
> > + * @igt_kmemleak_file: update the path to kmemleak file to enable unit testing
> > + *
> > + * Check if kmemleak is enabled in the kernel, if debugfs is mounted and
> > + * if kmemleak file is present and readable.
> > + *
> > + * Returns: true if kmemleak is enabled, false otherwise.
> > + */
> > +bool igt_kmemleak_init(const char *unit_test_kmemleak_file)
> > +{
> > + FILE *fp;
> > +
> > + if (unit_test_kmemleak_file)
> > + snprintf(igt_kmemleak_file,
> > + sizeof(igt_kmemleak_file),
> > + "%s",
> > + unit_test_kmemleak_file);
> > +
> > + fp = fopen(igt_kmemleak_file, "r");
> > + if (!fp)
> > + return false;
> > +
> > + fclose(fp);
> > + return true;
> > +}
> > +
> > +/**
> > + * igt_kmemeak_cmd:
> > + * @cmd: command to send to kmemleak
> > + *
> > + * Send a command to kmemleak.
> > + *
> > + * Returns: true if sending the command was successful, false otherwise.
> > + */
> > +bool igt_kmemleak_cmd(const char *cmd)
> > +{
> > + FILE *fp;
> > +
> > + fp = fopen(igt_kmemleak_file, "r+");
> > + if (!fp)
> > + return false;
> > +
> > + if (fwrite(cmd, 1, strlen(cmd), fp) != strlen(cmd)) {
> > + fclose(fp);
> > + return false;
> > + }
>
> Maybe we can reduce the number of fclose calls by capturing the fwrite return value:
> """
> bool igt_kmemleak_cmd(const char *cmd)
> {
> FILE *fp;
> size_t wlen;
>
> fp = fopen(igt_kmemleak_file, "r+");
> if (!fp)
> return false;
>
> wlen = fwrite(cmd, 1, strlen(cmd), fp);
> fclose(fp);
>
> return wlen == strlen(cmd);
> }
> """
> Though that's not strictly necessary here.
>
> > +
> > + fclose(fp);
> > + return true;
> > +}
> > +
> > +/**
> > + * igt_kmemeak_scan:
> > + *
> > + * Trigger an immediate scan on kmemleak.
> > + *
> > + * Returns: true if sending the command to scan was successful, false otherwise.
> > + */
> > +bool igt_kmemleak_scan(void)
> > +{
> > + return igt_kmemleak_cmd("scan");
> > +}
> > +
> > +/**
> > + * igt_kmemeak_clear:
> > + *
> > + * Trigger an immediate clear on kmemleak.
> > + *
> > + * Returns: true if sending the command to clear was successful, false
> > + * otherwise.
> > + */
> > +bool igt_kmemleak_clear(void)
> > +{
> > + return igt_kmemleak_cmd("clear");
> > +}
> > +
> > +/**
> > + * igt_kmemeak_found_leaks:
> > + *
> > + * Check if kmemleak found any leaks.
> > + *
> > + * Returns: true if kmemleak found any leaks, false otherwise.
> > + */
> > +bool igt_kmemleak_found_leaks(void)
> > +{
> > + FILE *fp;
> > + char buf[1];
> > +
> > + fp = fopen(igt_kmemleak_file, "r");
> > + if (!fp)
> > + return false;
> > +
> > + if (fread(buf, 1, 1, fp) <= 0) {
> > + fclose(fp);
> > + return false;
> > + }
>
> Same comment as with igt_kmemleak_cmd above:
> """
> bool igt_kmemleak_found_leak(void)
> {
> FILE *fp;
> char buf[1];
> size_t rlen;
>
> fp = fopen(igt_kmemleak_file, "r");
> if (!fp)
> return false;
>
> rlen = fread(buf, 1, 1, fp);
> fclose(fp);
>
> return rlen > 0;
> }
> """
> Though, again, this isn't strictly necessary.
>
> > +
> > + /* fread() did read 1 char */
> > + fclose(fp);
> > + return true;
> > +}
> > +
> > +/**
> > + * igt_kmemeak_cp_leaks_file:
> > + * @dst: path of destination file including the file name
> > + *
> > + * Copy kmemleak file to dst.
> > + *
> > + * Returns: true if copy was successful, false otherwise
> > + */
> > +bool igt_kmemleak_cp_leaks_file(const char *dst)
> > +{
> > + FILE *src, *dest;
> > + char buf[1024];
> > + size_t n;
> > +
> > + src = fopen(igt_kmemleak_file, "r");
> > + if (!src)
> > + return false;
> > +
> > + dest = fopen(dst, "w");
> > + if (!dest) {
> > + fclose(src);
> > + return false;
> > + }
> > +
> > + while ((n = fread(buf, 1, sizeof(buf), src)) > 0)
> > + fwrite(buf, 1, n, dest);
> > +
> > + fclose(src);
> > + fclose(dest);
>
> I think these two fclose calls should swap places for style reasons, but it's also
> not particularly important.
>
> > + return true;
> > +}
> > diff --git a/lib/igt_kmemleak.h b/lib/igt_kmemleak.h
> > new file mode 100644
> > index 000000000..a65f31810
> > --- /dev/null
> > +++ b/lib/igt_kmemleak.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: MIT
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#ifndef IGT_KMEMLEAK_H
> > +#define IGT_KMEMLEAK_H
> > +
> > +#include <stdbool.h>
> > +
> > +/**
> > + * igt_kmemeak_init:
> > + * @igt_kmemleak_file: update the path to kmemleak file to enable unit testing
> > + *
> > + * Check if kmemleak is enabled in the kernel, if debugfs is mounted and
> > + * if kmemleak file is present and readable.
> > + *
> > + * Returns: true if kmemleak is enabled, false otherwise.
> > + */
>
> I don't think we need comments like this for both the implementation
> and the header file. The former should be sufficient.
>
> > +bool igt_kmemleak_init(const char *unit_test_kmemleak_file);
> > +
> > +/**
> > + * igt_kmemeak_cmd:
> > + * @cmd: command to send to kmemleak
> > + *
> > + * Send a command to kmemleak.
> > + *
> > + * Returns: true if sending the command was successful, false otherwise.
> > + */
> > +bool igt_kmemleak_cmd(const char *cmd);
> > +
> > +/**
> > + * igt_kmemeak_scan:
> > + *
> > + * Trigger an immediate scan on kmemleak.
> > + *
> > + * Returns: true if sending the command to scan was successful, false otherwise.
> > + */
> > +bool igt_kmemleak_scan(void);
> > +
> > +/**
> > + * igt_kmemeak_clear:
> > + *
> > + * Trigger an immediate clear on kmemleak.
> > + *
> > + * Returns: true if sending the command to clear was successful, false
> > + * otherwise.
> > + */
> > +bool igt_kmemleak_clear(void);
> > +
> > +/**
> > + * igt_kmemeak_found_leaks:
> > + *
> > + * Check if kmemleak found any leaks.
> > + *
> > + * Returns: true if kmemleak found any leaks, false otherwise.
> > + */
> > +bool igt_kmemleak_found_leaks(void);
> > +
> > +/**
> > + * igt_kmemeak_cp_leaks_file:
> > + * @dst: path of destination file including the file name
> > + *
> > + * Copy kmemleak file to dst.
> > + *
> > + * Returns: true if copy was successful, false otherwise
> > + */
> > +bool igt_kmemleak_cp_leaks_file(const char *dst);
> > +
> > +#endif /* IGT_KMEMLEAK_H */
> > diff --git a/lib/meson.build b/lib/meson.build
> > index 640513e6c..24423ef37 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -96,6 +96,7 @@ lib_sources = [
> > 'igt_dummyload.c',
> > 'igt_store.c',
> > 'uwildmat/uwildmat.c',
> > + 'igt_kmemleak.c',
> > 'igt_kmod.c',
> > 'igt_ktap.c',
> > 'igt_panfrost.c',
> > diff --git a/lib/tests/igt_kmemleak.c b/lib/tests/igt_kmemleak.c
> > new file mode 100644
> > index 000000000..f5bf5ca07
> > --- /dev/null
> > +++ b/lib/tests/igt_kmemleak.c
> > @@ -0,0 +1,291 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#include <ctype.h>
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <zlib.h>
> > +
> > +#include "igt_core.h"
> > +#include "igt_kmemleak.h"
> > +
> > +const char *kmemleak_file_example =
> > +"unreferenced object 0xffff888102a2e638 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 00 00 00 00 00 00 00 00 0d 01 a2 00 00 00 00 00 ................\n"
> > +" f0 7c 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .|..............\n"
> > +" backtrace (crc 2df71a7e):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2ed18 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 38 e6 a2 02 81 88 ff ff 0d 11 2d 00 00 00 00 00 8.........-.....\n"
> > +" f2 7c 03 00 00 c9 ff ff 58 ea a2 02 81 88 ff ff .|......X.......\n"
> > +" backtrace (crc ec2a8bdc):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2ea58 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 38 e6 a2 02 81 88 ff ff 0d 01 a0 00 00 00 00 00 8...............\n"
> > +" f6 7c 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .|..............\n"
> > +" backtrace (crc f911c0d1):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e428 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 58 ea a2 02 81 88 ff ff 0d 01 35 00 00 00 00 00 X.........5.....\n"
> > +" fc 7c 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .|..............\n"
> > +" backtrace (crc cb8aaffd):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e008 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 28 e4 a2 02 81 88 ff ff 0d 01 2d 00 00 00 00 00 (.........-.....\n"
> > +" fc 7c 03 00 00 c9 ff ff c8 e2 a2 02 81 88 ff ff .|..............\n"
> > +" backtrace (crc 7f883e78):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2b9e5>] acpi_ps_get_next_namepath+0x1f5/0x390\n"
> > +" [<ffffffff81c2cc15>] acpi_ps_parse_loop+0x4a5/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e2c8 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 28 e4 a2 02 81 88 ff ff 0d 01 73 00 00 00 00 00 (.........s.....\n"
> > +" 00 7d 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .}..............\n"
> > +" backtrace (crc 338c016):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e378 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" c8 e2 a2 02 81 88 ff ff 0d 01 0d 00 00 00 00 00 ................\n"
> > +" 01 7d 03 00 00 c9 ff ff 98 e7 a2 02 81 88 ff ff .}..............\n"
> > +" backtrace (crc 665fb8a7):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e798 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 7c8 e2 a2 02 81 88 ff ff 0d 01 98 00 00 00 00 00 ................\n"
> > +" 1b 7d 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .}..............\n"
> > +" backtrace (crc b7a23a1c):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
> > +"unreferenced object 0xffff888102a2e0b8 (size 80):\n"
> > +" comm \"swapper/0\", pid 1, jiffies 4294672730\n"
> > +" hex dump (first 32 bytes):\n"
> > +" 98 e7 a2 02 81 88 ff ff 0d 01 2d 00 00 00 00 00 ..........-.....\n"
> > +" 1c 7d 03 00 00 c9 ff ff 00 00 00 00 00 00 00 00 .}..............\n"
> > +" backtrace (crc 14d67a9c):\n"
> > +" [<ffffffff824cd71b>] kmemleak_alloc+0x4b/0x80\n"
> > +" [<ffffffff814e169b>] kmem_cache_alloc_noprof+0x2ab/0x370\n"
> > +" [<ffffffff81c2f4dc>] acpi_ps_alloc_op+0xdc/0xf0\n"
> > +" [<ffffffff81c2d650>] acpi_ps_create_op+0x1c0/0x400\n"
> > +" [<ffffffff81c2c8dc>] acpi_ps_parse_loop+0x16c/0xa60\n"
> > +" [<ffffffff81c2e94f>] acpi_ps_parse_aml+0x22f/0x5f0\n"
> > +" [<ffffffff81c2fa82>] acpi_ps_execute_method+0x152/0x380\n"
> > +" [<ffffffff81c233ed>] acpi_ns_evaluate+0x31d/0x5e0\n"
> > +" [<ffffffff81c2a606>] acpi_evaluate_object+0x206/0x490\n"
> > +" [<ffffffff81bf1202>] __acpi_power_off.isra.0+0x22/0x70\n"
> > +" [<ffffffff81bf275b>] acpi_turn_off_unused_power_resources+0xbb/0xf0\n"
> > +" [<ffffffff83867799>] acpi_scan_init+0x119/0x290\n"
> > +" [<ffffffff8386711a>] acpi_init+0x23a/0x590\n"
> > +" [<ffffffff81002c71>] do_one_initcall+0x61/0x3d0\n"
> > +" [<ffffffff837dce32>] kernel_init_freeable+0x3e2/0x680\n"
> > +" [<ffffffff824ca53b>] kernel_init+0x1b/0x170\n";
> > +
> > +static unsigned long crc32_file(int fd)
> > +{
> > + unsigned long crc;
> > + char buf[1024];
> > + ssize_t n;
> > +
> > + crc = crc32(0, Z_NULL, 0);
> > + while ((n = read(fd, buf, sizeof(buf))) > 0)
> > + crc = crc32(crc, (const Bytef *)buf, n);
> > +
> > + return crc;
> > +}
> > +
> > +static void test_igt_kmemleak_cp_leaks_file(void)
> > +{
> > + char dst_file_path[256] = "/tmp/igt_kmemleak_dstfile_XXXXXX";
> > + int fd;
> > + unsigned long crc;
> > +
> > + fd = mkstemp(dst_file_path);
> > + close(fd);
> > +
> > + igt_assert(igt_kmemleak_cp_leaks_file(dst_file_path));
> > +
> > + /* Test that both files have the same size */
> > + fd = open(dst_file_path, O_RDONLY);
> > + igt_assert(fd >= 0);
> > + igt_assert(lseek(fd, 0, SEEK_END) == strlen(kmemleak_file_example));
> > + close(fd);
> > +
> > + /* Test that both files have the same content */
> > + crc = crc32(0, Z_NULL, 0);
> > + crc = crc32(crc,
> > + (const Bytef *)kmemleak_file_example,
> > + strlen(kmemleak_file_example));
Did you miss the changes to compare the CRC?
Regards,
Bala
> > +
> > + fd = open(dst_file_path, O_RDONLY);
> > + igt_assert(fd >= 0);
> > + igt_info(" dst_file: %s\n", dst_file_path);
> > + close(fd);
> > +}
> > +
> > +static void test_empty_file(void)
> > +{
> > + char tmp_file_path[256] = "/tmp/igt_kmemleak_test_XXXXXX";
> > + int fd;
> > +
> > + fd = mkstemp(tmp_file_path);
> > + igt_assert(fd >= 0);
> > + close(fd);
> > +
> > + /* Set the path for the unit testing file */
> > + igt_assert(igt_kmemleak_init(tmp_file_path));
> > + igt_assert(!igt_kmemleak_found_leaks());
> > +}
> > +
> > +static void test_non_empty_file(void)
> > +{
> > + char tmp_file_path[256] = "/tmp/igt_kmemleak_test_XXXXXX";
> > + int fd;
> > +
> > + fd = mkstemp(tmp_file_path);
> > + igt_assert(fd >= 0);
> > + write(fd, kmemleak_file_example, strlen(kmemleak_file_example));
> > +
> > + /* Set the path for the unit testing file */
> > + igt_assert(igt_kmemleak_init(tmp_file_path));
> > +
> > + /* Test igt_kmemleak_found_leaks() that should return true */
> > + igt_assert(igt_kmemleak_found_leaks());
>
> test_empty_file and test_non_empty_file could probably be consolidated into a single
> "test_file" or "test_kmemleak_file" test with a Boolean determining if we want to write
> to the test file or not:
> """
> static void test_kmemleak_file(bool write)
> {
> char tmp_file_path[256] = "/tmp/igt_kmemleak_test_XXXXXX";
> int fd = mkstemp(tmp_file_path);
> igt_assert(fd >= 0);
>
> if (write)
> write(fd, kmemleak_file_example, strlen(kmemleak_file_example));
>
> /* Set the path for the unit testing file */
> igt_assert(igt_kmemleak_init(tmp_file_path));
>
> /* Test igt_kmemleak_found_leaks() returns true if written */
> igt_assert(igt_kmemleak_found_leaks() == write);
>
> close(fd);
> }
> ...
> igt_simple_main
> {
> test_kmemleak_file(false);
> test_kmemleak_file(true);
> test_igt_kmemleak_cp_leaks_file();
> }
> """
>
> > +}
> > +
> > +igt_simple_main
> > +{
> > + test_empty_file();
> > + test_non_empty_file();
> > + test_igt_kmemleak_cp_leaks_file();
>
> It might be better to separate these out into their own subtests:
> """
> igt_subtest_f("empty-file")
> test_empty_file();
> igt_subtest_f("non-empty-file")
> test_non_empty_file();
> igt_subtest_f("cp-leaks-file")
> test_igt_kmemleak_cp_leaks_file();
> """
>
> Though looking at it a bit closer, I see that test_igt_kmemleak_cp_leaks_file depends
> on the completion of test_non_empty_file to run properly? Maybe we should separate
> the test out into chunks?
>
> Let me try prototyping it here:
> """
> --- Original file up to end of crc32_file function is unchanged ---
>
> static void init_kmemleak_file(bool write)
> {
> --- Same as test_kmemleak_file from earlier suggestion ---
> }
>
> static void test_kmemleak_cp_leaks_file(void)
> {
> char dst_file_path[256] = "/tmp/igt_kmemleak_dstfile_XXXXXX";
> int fd;
> unsigned long crc;
>
> init_kmemleak_file(true);
>
> fd = mkstemp(dst_file_path);
> --- Rest of test is unchanged after this point ---
> }
>
> igt_main
> {
> igt_subtest_f("empty")
> init_kmemleak_file(false);
> igt_subtest_f("non-empty")
> init_kmemleak_file(true);
> igt_subtest_f("cp-leaks-file")
> test_kmemleak_cp_leaks_file();
> }
> """
>
> -Jonathan Cavitt
>
> > +}
> > diff --git a/lib/tests/meson.build b/lib/tests/meson.build
> > index df8092638..bcbea9ba4 100644
> > --- a/lib/tests/meson.build
> > +++ b/lib/tests/meson.build
> > @@ -15,6 +15,7 @@ lib_tests = [
> > 'igt_ktap_parser',
> > 'igt_list_only',
> > 'igt_invalid_subtest_name',
> > + 'igt_kmemleak',
> > 'igt_nesting',
> > 'igt_no_exit',
> > 'igt_runnercomms_packets',
> > --
> > 2.34.1
> >
> >
More information about the igt-dev
mailing list