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.
>>>>>
>>>>> 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
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
>
>