Re: kvzalloc vs kvcalloc

[Deepak R Varma]

On Tue, Dec 20, 2022 at 08:13:19AM -0600, Gustavo A. R. Silva wrote:
>
>
> On 12/20/22 01:48, Julia Lawall wrote:
> >
> >
> > On Tue, 20 Dec 2022, Deepak R Varma wrote:
> >
> > > On Tue, Dec 20, 2022 at 08:39:09AM +0100, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 20 Dec 2022, Deepak R Varma wrote:
> > > >
> > > > > On Tue, Dec 20, 2022 at 07:08:24AM +0100, Julia Lawall wrote:
> > > > > >
> > > > > >
> > > > > > On Tue, 20 Dec 2022, Deepak R Varma wrote:
> > > > > >
> > > > > > > Hello Gustavo and Julia,
> > > > > > > I was working on building a patch proposal using the kvmalloc.cocci file for a
> > > > > > > driver. The recommendation from the semantic patch is to use kvzalloc instead of
> > > > > > > a fallback memory allocation model. Please see my patch submitted here [1].
> > > > > > >
> > > > > > > I also found another patch submitted by Gustavo [2] which suggests using
> > > > > > > kvcalloc instead of kvzalloc. Unfortunately, I was not able to understand the
> > > > > > > reasons/advantages using kvcalloc over kvzalloc.
>
> Look for the definitions of those functions and try to understand their differences.
> In many cases you have go down the rabbit hole, but you should be able to get a good
> grasp of the thing in question before hitting the bottom. :)
>
> Look for a couple of instances in the codebase where those functions are being used
> and try to understand a bit of the context around them. In some cases reading the
> commit logs is necessary.

Hello Gustavo,
Thank you very much for the suggestion here. I will get deeper into the codebase
and try to self learn. Your advise on reading the past commit logs is useful as
well.

Thank you again!

./drv

>
> > > > > >
> > > > > > The calloc variants are for zeroed arrays.  zalloc variants just zero.
> > > > >
> > > > > Thank you Julia and sorry to have missed the references in my email:
> > > >
> > > > In Gustavo's case, the array has a certain number of elements of a certain
> > > > size.  I don't know if you have both pieces of information in your case.
> > > > calloc functions take them in separately, and do the multiplication in a
> > > > way that checks for overflows.
> > >
> > > That is correct and I do have both the pieces, the size and number. This
> > > actually further optimizes the code. We can eliminate the array_size variable
> > > with the kvcalloc implementation. It is not used beyond the memory allocation.
> > >
> > > Please this code snip:
> > >
> > > 	853          int count = size >> PAGE_SHIFT;
> > > 	   1         int array_size = count * sizeof(struct page *);
> > > 	   2         int i = 0;
> > > 	   3         int order_idx = 0;
> > > 	   4
> > > 	   5         pages = kvzalloc(array_size, GFP_KERNEL);
> > > 	   6         if (!pages)
> > > 	   7                 return NULL;
> > >
> > > Thank you for your advise. I will wait to see Gustavo has any further guidance.
> > > I will send in a revision to my patch accordingly.
> >
> > Great.  A calloc function definitely looks like a good choice here.
>
> As Julia suggested, and as you may had realized already, the calloc function is the
> way to go, in this case.
>
> --
> Gustavo
>
>