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

[Tom Rix]

On 3/13/22 9:09 AM, Joe Perches wrote:
> On Sun, 2022-03-13 at 07:08 -0700, trix@xxxxxxxxxx wrote:
> > From: Tom Rix <trix@xxxxxxxxxx>
> >
> > 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

>