On Tue, Jul 07, 2020 at 12:26:52PM -0500, Uriel Guajardo wrote:
On Mon, Jul 6, 2020 at 6:17 PM Qian Cai cai@lca.pw wrote:
On Mon, Jul 06, 2020 at 05:48:21PM -0500, Uriel Guajardo wrote:
On Mon, Jul 6, 2020 at 4:39 PM Qian Cai cai@lca.pw wrote:
On Mon, Jul 06, 2020 at 09:13:09PM +0000, Uriel Guajardo wrote:
From: Uriel Guajardo urielguajardo@google.com
Integrate kmemleak into the KUnit testing framework.
Kmemleak will now fail the currently running KUnit test case if it finds any memory leaks.
The minimum object age for reporting is set to 0 msecs so that leaks are not ignored if the test case finishes too quickly.
Signed-off-by: Uriel Guajardo urielguajardo@google.com
include/linux/kmemleak.h | 11 +++++++++++ lib/Kconfig.debug | 26 ++++++++++++++++++++++++++ lib/kunit/test.c | 36 +++++++++++++++++++++++++++++++++++- mm/kmemleak.c | 27 +++++++++++++++++++++------ 4 files changed, 93 insertions(+), 7 deletions(-)
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h index 34684b2026ab..0da427934462 100644 --- a/include/linux/kmemleak.h +++ b/include/linux/kmemleak.h @@ -35,6 +35,10 @@ extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref; extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref; extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
+extern ssize_t kmemleak_write(struct file *file,
const char __user *user_buf,size_t size, loff_t *ppos);static inline void kmemleak_alloc_recursive(const void *ptr, size_t size, int min_count, slab_flags_t flags, gfp_t gfp) @@ -120,6 +124,13 @@ static inline void kmemleak_ignore_phys(phys_addr_t phys) { }
+static inline ssize_t kmemleak_write(struct file *file,
const char __user *user_buf,size_t size, loff_t *ppos)+{
return -1;+}
#endif /* CONFIG_DEBUG_KMEMLEAK */
#endif /* __KMEMLEAK_H */ diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 21d9c5f6e7ec..e9c492cb3f4d 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -602,6 +602,32 @@ config DEBUG_KMEMLEAK_MEM_POOL_SIZE fully initialised, this memory pool acts as an emergency one if slab allocations fail.
+config DEBUG_KMEMLEAK_MAX_TRACE
int "Kmemleak stack trace length"depends on DEBUG_KMEMLEAKdefault 16+config DEBUG_KMEMLEAK_MSECS_MIN_AGE
int "Minimum object age before reporting in msecs"depends on DEBUG_KMEMLEAKdefault 0 if KUNITdefault 5000+config DEBUG_KMEMLEAK_SECS_FIRST_SCAN
int "Delay before first scan in secs"depends on DEBUG_KMEMLEAKdefault 60+config DEBUG_KMEMLEAK_SECS_SCAN_WAIT
int "Delay before subsequent auto scans in secs"depends on DEBUG_KMEMLEAKdefault 600+config DEBUG_KMEMLEAK_MAX_SCAN_SIZE
int "Maximum size of scanned block"depends on DEBUG_KMEMLEAKdefault 4096Why do you make those configurable? I don't see anywhere you make use of them except DEBUG_KMEMLEAK_MSECS_MIN_AGE?
That's correct. Strictly speaking, only DEBUG_KMEMLEAK_MSECS_MIN_AGE is used to set a default when KUnit is configured.
There is no concrete reason why these other variables need to be configurable. At the time of writing this, it seemed to make the most sense to configure the other configuration options, given that I was already going to make MSECS_MIN_AGE configurable. It can definitely be taken out.
Even then, how setting DEBUG_KMEMLEAK_MSECS_MIN_AGE=0 not giving too many false positives? Kmemleak simply does not work that instantly.
I did not experience this issue, but I see your point.
An alternative that I was thinking about -- and one that is not in this patch -- is to wait DEBUG_KMEMLEAK_MSECS_MIN_AGE after each test case in a test suite, while leaving kmemleak's default value as is. I was hesitant to do this initially because many KUnit test cases run quick, so this may result in a lot of time just waiting. But if we leave it configurable, the user can change this as needed and deal with the possible false positives.
I doubt that is good idea. We don't really want people to start reporting those false positives to the MLs just because some kunit tests starts to flag them. It is wasting everyone's time. Reports from DEBUG_KMEMLEAK_MSECS_MIN_AGE=0 are simply trustful. I don't think there is a way around. Kmemleak was designed to have a lot of waitings/re-scans to be useful not even mentioning kfree_rcu() etc until it is redesigned...
I agree with your statement about false positives. Is your suggestion to not make MSECS_MIN_AGE configurable and have KUnit wait after each test case? Or are you saying that this will not work entirely? It seems like kmemleak should be able to work in some fashion under KUnit, since it has specific documentation over testing parts of code (https://www.kernel.org/doc/html/latest/dev-tools/kmemleak.html#testing-speci...).
It is going to be tough. It is normal that sometimes when there is a leak. It needs to rescan a few times to make sure it is stable. Sometimes, even the real leaks will take quite a while to show up.