Re: sparse problem with Linux kernel v5.5

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Thu, Feb 06, 2020 at 10:25:39AM -0800, Randy Dunlap wrote:
> 
> All of the oom-killer instances list sphinx-build as running, and I often
> have OOM problems using it (make htmldocs), so that may be a factor.
> 
> This morning sparse is handling bpf_sk_storage.c OK (no sphinx-build running).

OK, make sense.
However, I thought that the 5+seconds of runtime with 2.9Gb of memory
consumption I reported earlier was somehow excessive. So, I looked
at the preprocessed file and my editor (and several other tools) chocked
near the end ... It appears that one line is 2.8Mb on a total of 6.2MB
and contains 28968 times the expression for num_possible_cpus().

The origin of this is situted at line 647:
	smap->bucket_log = max_t(u32, 1, ilog2(roundup_pow_of_two(...));
because ilog2() is an 'interesting' macro which is already expanded
inside roundup_pow_of_two().

This exists since the introduction of this file in commit
    6ac99e8f23d4 bpf: Introduce bpf sk local storage
but back then it made sparse consume only about 500Mb of memory on it.
Use of the macro max_t make things even worse in commit 
    85749218e3a6 ("bpf: Fix out of bounds memory access in bpf_sk_storage")

To illustrate the situation, the following patch cuts sparse memory consumption
down to 109Mb in 0.3s:

diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 458be6b3eda9..bdf4a1a256c3 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -644,7 +644,7 @@ static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
 	bpf_map_init_from_attr(&smap->map, attr);
 
 	/* Use at least 2 buckets, select_bucket() is undefined behavior with 1 bucket */
-	smap->bucket_log = max_t(u32, 1, ilog2(roundup_pow_of_two(num_possible_cpus())));
+	smap->bucket_log = max_t(u32, 1, ilog2(__roundup_pow_of_two(num_possible_cpus())));
 	nbuckets = 1U << smap->bucket_log;
 	cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
 

but a better patch should, I think, directly use ilog2() and avoid the roundup.

Cheers,
-- Luc