linux-kernel - Re: [PATCH] checkpatch: warn that small allocs should be combined

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d51c3c16-21fa-01c7-3faf-e96eb70c4721@redhat.com>
Date:   Sun, 13 Mar 2022 10:25:38 -0700
From:   Tom Rix <trix@...hat.com>
To:     Joe Perches <joe@...ches.com>, apw@...onical.com,
        dwaipayanray1@...il.com, lukas.bulwahn@...il.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] checkpatch: warn that small allocs should be combined


On 3/13/22 9:09 AM, Joe Perches wrote:
> On Sun, 2022-03-13 at 07:08 -0700, trix@...hat.com wrote:
>> From: Tom Rix <trix@...hat.com>
>>
>> A memory allocation has overhead.  When a
>> small allocation is made the overhead dominates.
>> By combining the fixed sized small allocations
>> with others, the memory usage can be reduced
>> by eliminating the overhead of the small allocs.
> This will generate false positives as small allocs are
> sometimes required for usb dma.
>
> How many of these "small allocs" _could_ be combined and under
> what circumstance?
>
> Can you show me a current example in the kernel where this
> is useful?

Tracing what the memory is used for is not easy.

And opens a can of worms.

Most/all of the alloc use only GFP_KERNEL.

If this allocs implicitly align / size suites dma which i am

guessing is (void *) aligned/size then then there will be some

cases of overalignment.

The addition of a GFP_DMA could indicate the memory was to be dma-ed, 
but cause other breakage.

So there is not a good way currently for checkpatch to figure this out.

>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -7076,6 +7076,12 @@ sub process {
>>   			     "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr);
>>   		}
>>   
>> +# check for small allocs
>> +		if ($line =~ /\b(?:kv|k|v)[zm]alloc\s*\(\s*(\d|sizeof\s*\([su](8|16|32)s*\))\s*,/) {
>> +			WARN("SMALL_ALLOC",
>> +			     "Small allocs should be combined\n" . $herecurr);
>> +		}
>> +
> Couple more comments:
>
> Anyone using vmalloc variants for a small alloc is confused.
> What defines "small"?
> Why would a single decimal like 8 be small, but say 16 would not be?
>
> checkpatch has a couple of regexes that could be useful here
>
> Maybe instead of sizeof(your regex) use
>
> 	sizeof\s*\(\s*(?:\d|$C90_int_types|$typeTypedefs)\s*,
>
> as that will find more "small" uses of individual types like
> "unsigned long", __s32, u_int_16, etc...
>
ok

Tom

>