[PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Pintu Kumar]

With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
based on their needs. For some application needs, this can grow as large
as 4MB that is, 2^10 order pages, which is unlikely to succeed with
kzalloc.
Even the default (256KB) may fail on highly fragmented system.
Further without COMPACTION enabled, the situation can be worse.

Moreover, for this heap allocation, we don't need physical contiguous
memory. Thus vmalloc may be sufficient here.
But for performance benefit we may like to stick to kmalloc.

Thus, it is better to replace kzalloc with kvzalloc which will first try
to use kmalloc and if fails, fallback to vmalloc.

Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
Signed-off-by: sunshilong <sunshilong369@gmail.com>
Tested-by: sunshilong <sunshilong369@gmail.com>
---
 kernel/cobalt/heap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
index d01a2e0..312b41f 100644
--- a/kernel/cobalt/heap.c
+++ b/kernel/cobalt/heap.c
@@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
 	xnlock_init(&heap->lock);
 
 	nrpages = size >> XNHEAP_PAGE_SHIFT;
-	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
+	heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
 				GFP_KERNEL);
 	if (heap->pagemap == NULL)
 		return -ENOMEM;
@@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
 	nrheaps--;
 	xnvfile_touch_tag(&vfile_tag);
 	xnlock_put_irqrestore(&nklock, s);
-	kfree(heap->pagemap);
+	kvfree(heap->pagemap);
 }
 EXPORT_SYMBOL_GPL(xnheap_destroy);
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[pintu]

On 2020-07-31 23:34, Pintu Kumar wrote:
> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
> based on their needs. For some application needs, this can grow as 
> large
> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> kzalloc.
> Even the default (256KB) may fail on highly fragmented system.
> Further without COMPACTION enabled, the situation can be worse.
> 
> Moreover, for this heap allocation, we don't need physical contiguous
> memory. Thus vmalloc may be sufficient here.
> But for performance benefit we may like to stick to kmalloc.
> 
> Thus, it is better to replace kzalloc with kvzalloc which will first 
> try
> to use kmalloc and if fails, fallback to vmalloc.
> 
> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> Signed-off-by: sunshilong <sunshilong369@gmail.com>
> Tested-by: sunshilong <sunshilong369@gmail.com>
> ---
>  kernel/cobalt/heap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> index d01a2e0..312b41f 100644
> --- a/kernel/cobalt/heap.c
> +++ b/kernel/cobalt/heap.c
> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void
> *membase, size_t size)
>  	xnlock_init(&heap->lock);
> 
>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> +	heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>  				GFP_KERNEL);
>  	if (heap->pagemap == NULL)
>  		return -ENOMEM;
> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>  	nrheaps--;
>  	xnvfile_touch_tag(&vfile_tag);
>  	xnlock_put_irqrestore(&nklock, s);
> -	kfree(heap->pagemap);
> +	kvfree(heap->pagemap);
>  }
>  EXPORT_SYMBOL_GPL(xnheap_destroy);


Hi,

Is there any feedback or comments on the above patch ?


Thanks,
Pintu

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[pintu]

On 2020-08-07 10:26, pintu@codeaurora.org wrote:
> On 2020-07-31 23:34, Pintu Kumar wrote:
>> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
>> based on their needs. For some application needs, this can grow as 
>> large
>> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
>> kzalloc.
>> Even the default (256KB) may fail on highly fragmented system.
>> Further without COMPACTION enabled, the situation can be worse.
>> 
>> Moreover, for this heap allocation, we don't need physical contiguous
>> memory. Thus vmalloc may be sufficient here.
>> But for performance benefit we may like to stick to kmalloc.
>> 
>> Thus, it is better to replace kzalloc with kvzalloc which will first 
>> try
>> to use kmalloc and if fails, fallback to vmalloc.
>> 
>> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
>> Signed-off-by: sunshilong <sunshilong369@gmail.com>
>> Tested-by: sunshilong <sunshilong369@gmail.com>
>> ---
>>  kernel/cobalt/heap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>> index d01a2e0..312b41f 100644
>> --- a/kernel/cobalt/heap.c
>> +++ b/kernel/cobalt/heap.c
>> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void
>> *membase, size_t size)
>>  	xnlock_init(&heap->lock);
>> 
>>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
>> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>> +	heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>  				GFP_KERNEL);
>>  	if (heap->pagemap == NULL)
>>  		return -ENOMEM;
>> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>>  	nrheaps--;
>>  	xnvfile_touch_tag(&vfile_tag);
>>  	xnlock_put_irqrestore(&nklock, s);
>> -	kfree(heap->pagemap);
>> +	kvfree(heap->pagemap);
>>  }
>>  EXPORT_SYMBOL_GPL(xnheap_destroy);
> 
> 
> Hi,
> 
> Is there any feedback or comments on the above patch ?
> 
> 
> Thanks,
> Pintu

Hi,

Is there any further update of this ?
Just a reminder to check....


Regards,
Pintu

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Philippe Gerum]

On 8/10/20 9:09 AM, pintu@codeaurora.org wrote:
> On 2020-08-07 10:26, pintu@codeaurora.org wrote:
>> On 2020-07-31 23:34, Pintu Kumar wrote:
>>> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
>>> based on their needs. For some application needs, this can grow as large
>>> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
>>> kzalloc.
>>> Even the default (256KB) may fail on highly fragmented system.
>>> Further without COMPACTION enabled, the situation can be worse.
>>>
>>> Moreover, for this heap allocation, we don't need physical contiguous
>>> memory. Thus vmalloc may be sufficient here.
>>> But for performance benefit we may like to stick to kmalloc.
>>>
>>> Thus, it is better to replace kzalloc with kvzalloc which will first try
>>> to use kmalloc and if fails, fallback to vmalloc.
>>>
>>> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
>>> Signed-off-by: sunshilong <sunshilong369@gmail.com>
>>> Tested-by: sunshilong <sunshilong369@gmail.com>
>>> ---
>>>  kernel/cobalt/heap.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>>> index d01a2e0..312b41f 100644
>>> --- a/kernel/cobalt/heap.c
>>> +++ b/kernel/cobalt/heap.c
>>> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void
>>> *membase, size_t size)
>>>      xnlock_init(&heap->lock);
>>>
>>>      nrpages = size >> XNHEAP_PAGE_SHIFT;
>>> -    heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>> +    heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>>                  GFP_KERNEL);
>>>      if (heap->pagemap == NULL)
>>>          return -ENOMEM;
>>> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>>>      nrheaps--;
>>>      xnvfile_touch_tag(&vfile_tag);
>>>      xnlock_put_irqrestore(&nklock, s);
>>> -    kfree(heap->pagemap);
>>> +    kvfree(heap->pagemap);
>>>  }
>>>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>>
>>
>> Hi,
>>
>> Is there any feedback or comments on the above patch ?
>>
>>
>> Thanks,
>> Pintu
> 
> Hi,
> 
> Is there any further update of this ?
> Just a reminder to check....
> 

This patch makes sense to me. I'll leave it to the current maintainer to
decide whether it should be merged.

Thanks,

-- 
Philippe.

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 31.07.20 20:04, Pintu Kumar via Xenomai wrote:
> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
> based on their needs. For some application needs, this can grow as large
> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> kzalloc.
> Even the default (256KB) may fail on highly fragmented system.
> Further without COMPACTION enabled, the situation can be worse.
> 
> Moreover, for this heap allocation, we don't need physical contiguous
> memory. Thus vmalloc may be sufficient here.
> But for performance benefit we may like to stick to kmalloc.
> 
> Thus, it is better to replace kzalloc with kvzalloc which will first try
> to use kmalloc and if fails, fallback to vmalloc.
> 
> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> Signed-off-by: sunshilong <sunshilong369@gmail.com>
> Tested-by: sunshilong <sunshilong369@gmail.com>
> ---
>  kernel/cobalt/heap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> index d01a2e0..312b41f 100644
> --- a/kernel/cobalt/heap.c
> +++ b/kernel/cobalt/heap.c
> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
>  	xnlock_init(&heap->lock);
>  
>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> +	heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>  				GFP_KERNEL);
>  	if (heap->pagemap == NULL)
>  		return -ENOMEM;
> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>  	nrheaps--;
>  	xnvfile_touch_tag(&vfile_tag);
>  	xnlock_put_irqrestore(&nklock, s);
> -	kfree(heap->pagemap);
> +	kvfree(heap->pagemap);
>  }
>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>  
> 

Thanks, applied.

Jan

PS: Please make sure a "From:" line is at the top of your commit message
to avoid authorship mangling.

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 11.08.20 09:11, Jan Kiszka via Xenomai wrote:
> On 31.07.20 20:04, Pintu Kumar via Xenomai wrote:
>> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
>> based on their needs. For some application needs, this can grow as large
>> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
>> kzalloc.
>> Even the default (256KB) may fail on highly fragmented system.
>> Further without COMPACTION enabled, the situation can be worse.
>>
>> Moreover, for this heap allocation, we don't need physical contiguous
>> memory. Thus vmalloc may be sufficient here.
>> But for performance benefit we may like to stick to kmalloc.
>>
>> Thus, it is better to replace kzalloc with kvzalloc which will first try
>> to use kmalloc and if fails, fallback to vmalloc.
>>
>> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
>> Signed-off-by: sunshilong <sunshilong369@gmail.com>
>> Tested-by: sunshilong <sunshilong369@gmail.com>
>> ---
>>  kernel/cobalt/heap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>> index d01a2e0..312b41f 100644
>> --- a/kernel/cobalt/heap.c
>> +++ b/kernel/cobalt/heap.c
>> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
>>  	xnlock_init(&heap->lock);
>>  
>>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
>> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>> +	heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>  				GFP_KERNEL);
>>  	if (heap->pagemap == NULL)
>>  		return -ENOMEM;
>> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>>  	nrheaps--;
>>  	xnvfile_touch_tag(&vfile_tag);
>>  	xnlock_put_irqrestore(&nklock, s);
>> -	kfree(heap->pagemap);
>> +	kvfree(heap->pagemap);
>>  }
>>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>>  
>>
> 
> Thanks, applied.
> 
> Jan
> 
> PS: Please make sure a "From:" line is at the top of your commit message
> to avoid authorship mangling.
> 

Breaks 4.4 kernels. Please have a look:

https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[孙世龙 sunshilong]

Hi, Jan Kiszka, Pintu Kunmar

>Breaks 4.4 kernels. Please have a look:
>https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968
Sorry, I forgot.
It seems that kvzalloc is not available for linux-4.4.
So, this patch
(see https://xenomai.org/pipermail/xenomai/2020-July/043137.html) may be better.

For your convenience, here is the patch:
diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
index d01a2e0..f9d9a05 100644
--- a/kernel/cobalt/heap.c
+++ b/kernel/cobalt/heap.c
@@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void
*membase, size_t size)
        xnlock_init(&heap->lock);

        nrpages = size >> XNHEAP_PAGE_SHIFT;
-       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
-                               GFP_KERNEL);
+       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
+                               GFP_KERNEL | __GFP_ZERO);
        if (heap->pagemap == NULL)
                return -ENOMEM;

@@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
        nrheaps--;
        xnvfile_touch_tag(&vfile_tag);
        xnlock_put_irqrestore(&nklock, s);
-       kfree(heap->pagemap);
+       kvfree(heap->pagemap);
 }
 EXPORT_SYMBOL_GPL(xnheap_destroy);




On Tue, Aug 11, 2020 at 4:10 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 11.08.20 09:11, Jan Kiszka via Xenomai wrote:
> > On 31.07.20 20:04, Pintu Kumar via Xenomai wrote:
> >> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user that request any heap size
> >> based on their needs. For some application needs, this can grow as large
> >> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> >> kzalloc.
> >> Even the default (256KB) may fail on highly fragmented system.
> >> Further without COMPACTION enabled, the situation can be worse.
> >>
> >> Moreover, for this heap allocation, we don't need physical contiguous
> >> memory. Thus vmalloc may be sufficient here.
> >> But for performance benefit we may like to stick to kmalloc.
> >>
> >> Thus, it is better to replace kzalloc with kvzalloc which will first try
> >> to use kmalloc and if fails, fallback to vmalloc.
> >>
> >> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> >> Signed-off-by: sunshilong <sunshilong369@gmail.com>
> >> Tested-by: sunshilong <sunshilong369@gmail.com>
> >> ---
> >>  kernel/cobalt/heap.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> >> index d01a2e0..312b41f 100644
> >> --- a/kernel/cobalt/heap.c
> >> +++ b/kernel/cobalt/heap.c
> >> @@ -749,7 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
> >>      xnlock_init(&heap->lock);
> >>
> >>      nrpages = size >> XNHEAP_PAGE_SHIFT;
> >> -    heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> >> +    heap->pagemap = kvzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> >>                              GFP_KERNEL);
> >>      if (heap->pagemap == NULL)
> >>              return -ENOMEM;
> >> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
> >>      nrheaps--;
> >>      xnvfile_touch_tag(&vfile_tag);
> >>      xnlock_put_irqrestore(&nklock, s);
> >> -    kfree(heap->pagemap);
> >> +    kvfree(heap->pagemap);
> >>  }
> >>  EXPORT_SYMBOL_GPL(xnheap_destroy);
> >>
> >>
> >
> > Thanks, applied.
> >
> > Jan
> >
> > PS: Please make sure a "From:" line is at the top of your commit message
> > to avoid authorship mangling.
> >
>
> Breaks 4.4 kernels. Please have a look:
>
> https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 11.08.20 10:48, 孙世龙 sunshilong wrote:
> Hi, Jan Kiszka, Pintu Kunmar
> 
>> Breaks 4.4 kernels. Please have a look:
>> https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968
> Sorry, I forgot.
> It seems that kvzalloc is not available for linux-4.4.
> So, this patch
> (see https://xenomai.org/pipermail/xenomai/2020-July/043137.html) may be better.
> 
> For your convenience, here is the patch:
> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> index d01a2e0..f9d9a05 100644
> --- a/kernel/cobalt/heap.c
> +++ b/kernel/cobalt/heap.c
> @@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void
> *membase, size_t size)
>         xnlock_init(&heap->lock);
> 
>         nrpages = size >> XNHEAP_PAGE_SHIFT;
> -       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> -                               GFP_KERNEL);
> +       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
> +                               GFP_KERNEL | __GFP_ZERO);
>         if (heap->pagemap == NULL)
>                 return -ENOMEM;
> 
> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>         nrheaps--;
>         xnvfile_touch_tag(&vfile_tag);
>         xnlock_put_irqrestore(&nklock, s);
> -       kfree(heap->pagemap);
> +       kvfree(heap->pagemap);
>  }
>  EXPORT_SYMBOL_GPL(xnheap_destroy);
> 
> 

Could you send a v2 of this patch with the more exhaustive description
you had in v1?

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[孙世龙 sunshilong]

Hi, Jan Kiszka, Pintu Kunmar

With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, users that request any heap size
based on their needs. For some application needs, this can grow as large
as 4MB that is, 2^10 order pages, which is unlikely to succeed with
kzalloc.
Even the default (256KB) may fail on a highly fragmented system.
Further without COMPACTION enabled, the situation can be worse.

Moreover, for this heap allocation, we don't need physical contiguous
memory. Thus vmalloc may be sufficient here.
But for performance benefit, we may like to stick to malloc.

Thus, it is better to replace kzalloc with kvmalloc which will first try
to use kmalloc and if fails, fallback to vmalloc.

TIPS: Since kvzalloc is not available on Linux-4.4, it's better to use kvmalloc
to initialize the memory instead of kvzmalloc.

For a more detailed explanation of the problem and consequence involved in
the original code snippet, see
https://xenomai.org/pipermail/xenomai/2020-July/043137.html

Signed-off-by: sunshilong <sunshilong369@gmail.com> Pintu Kumar
<pintu@codeaurora.org>
Tested-by: sunshilong <sunshilong369@gmail.com>

---
 kernel/cobalt/heap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
index d01a2e0..f9d9a05 100644
--- a/kernel/cobalt/heap.c
+++ b/kernel/cobalt/heap.c
@@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void *membase,
size_t size)
        xnlock_init(&heap->lock);

        nrpages = size >> XNHEAP_PAGE_SHIFT;
-       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
-                               GFP_KERNEL);
+       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
+                               GFP_KERNEL | __GFP_ZERO);
        if (heap->pagemap == NULL)
                return -ENOMEM;

@@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
        nrheaps--;
        xnvfile_touch_tag(&vfile_tag);
        xnlock_put_irqrestore(&nklock, s);
-       kfree(heap->pagemap);
+       kvfree(heap->pagemap);
 }
 EXPORT_SYMBOL_GPL(xnheap_destroy);

On Tue, Aug 11, 2020 at 4:51 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 11.08.20 10:48, 孙世龙 sunshilong wrote:
> > Hi, Jan Kiszka, Pintu Kunmar
> >
> >> Breaks 4.4 kernels. Please have a look:
> >> https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968
> > Sorry, I forgot.
> > It seems that kvzalloc is not available for linux-4.4.
> > So, this patch
> > (see https://xenomai.org/pipermail/xenomai/2020-July/043137.html) may be better.
> >
> > For your convenience, here is the patch:
> > diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> > index d01a2e0..f9d9a05 100644
> > --- a/kernel/cobalt/heap.c
> > +++ b/kernel/cobalt/heap.c
> > @@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void
> > *membase, size_t size)
> >         xnlock_init(&heap->lock);
> >
> >         nrpages = size >> XNHEAP_PAGE_SHIFT;
> > -       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> > -                               GFP_KERNEL);
> > +       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
> > +                               GFP_KERNEL | __GFP_ZERO);
> >         if (heap->pagemap == NULL)
> >                 return -ENOMEM;
> >
> > @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
> >         nrheaps--;
> >         xnvfile_touch_tag(&vfile_tag);
> >         xnlock_put_irqrestore(&nklock, s);
> > -       kfree(heap->pagemap);
> > +       kvfree(heap->pagemap);
> >  }
> >  EXPORT_SYMBOL_GPL(xnheap_destroy);
> >
> >
>
> Could you send a v2 of this patch with the more exhaustive description
> you had in v1?
>
> Thanks,
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 11.08.20 13:44, Pintu Agarwal wrote:
> Oh sorry about this.
> I can send a new patch set with kvmalloc that can work on all kernel versions.
> 

Yes, please do so to avoid that things become to messy when trying to 
create that manually.

Thanks,
Jan

> Thanks,
> Pintu
> 
> On Tue, 11 Aug 2020 at 14:47, 孙世龙 sunshilong <sunshilong369@gmail.com> wrote:
>>
>> Hi, Jan Kiszka, Pintu Kunmar
>>
>> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, users that request any heap size
>> based on their needs. For some application needs, this can grow as large
>> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
>> kzalloc.
>> Even the default (256KB) may fail on a highly fragmented system.
>> Further without COMPACTION enabled, the situation can be worse.
>>
>> Moreover, for this heap allocation, we don't need physical contiguous
>> memory. Thus vmalloc may be sufficient here.
>> But for performance benefit, we may like to stick to malloc.
>>
>> Thus, it is better to replace kzalloc with kvmalloc which will first try
>> to use kmalloc and if fails, fallback to vmalloc.
>>
>> TIPS: Since kvzalloc is not available on Linux-4.4, it's better to use kvmalloc
>> to initialize the memory instead of kvzmalloc.
>>
>> For a more detailed explanation of the problem and consequence involved in
>> the original code snippet, see
>> https://xenomai.org/pipermail/xenomai/2020-July/043137.html
>>
>> Signed-off-by: sunshilong <sunshilong369@gmail.com> Pintu Kumar
>> <pintu@codeaurora.org>
>> Tested-by: sunshilong <sunshilong369@gmail.com>
>>
>> ---
>>   kernel/cobalt/heap.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>> index d01a2e0..f9d9a05 100644
>> --- a/kernel/cobalt/heap.c
>> +++ b/kernel/cobalt/heap.c
>> @@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void *membase,
>> size_t size)
>>          xnlock_init(&heap->lock);
>>
>>          nrpages = size >> XNHEAP_PAGE_SHIFT;
>> -       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>> -                               GFP_KERNEL);
>> +       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
>> +                               GFP_KERNEL | __GFP_ZERO);
>>          if (heap->pagemap == NULL)
>>                  return -ENOMEM;
>>
>> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>>          nrheaps--;
>>          xnvfile_touch_tag(&vfile_tag);
>>          xnlock_put_irqrestore(&nklock, s);
>> -       kfree(heap->pagemap);
>> +       kvfree(heap->pagemap);
>>   }
>>   EXPORT_SYMBOL_GPL(xnheap_destroy);
>>
>> On Tue, Aug 11, 2020 at 4:51 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>
>>> On 11.08.20 10:48, 孙世龙 sunshilong wrote:
>>>> Hi, Jan Kiszka, Pintu Kunmar
>>>>
>>>>> Breaks 4.4 kernels. Please have a look:
>>>>> https://travis-ci.com/github/xenomai-ci/xenomai/builds/179223968
>>>> Sorry, I forgot.
>>>> It seems that kvzalloc is not available for linux-4.4.
>>>> So, this patch
>>>> (see https://xenomai.org/pipermail/xenomai/2020-July/043137.html) may be better.
>>>>
>>>> For your convenience, here is the patch:
>>>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>>>> index d01a2e0..f9d9a05 100644
>>>> --- a/kernel/cobalt/heap.c
>>>> +++ b/kernel/cobalt/heap.c
>>>> @@ -749,8 +749,8 @@ int xnheap_init(struct xnheap *heap, void
>>>> *membase, size_t size)
>>>>          xnlock_init(&heap->lock);
>>>>
>>>>          nrpages = size >> XNHEAP_PAGE_SHIFT;
>>>> -       heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>>> -                               GFP_KERNEL);
>>>> +       heap->pagemap = kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
>>>> +                               GFP_KERNEL | __GFP_ZERO);
>>>>          if (heap->pagemap == NULL)
>>>>                  return -ENOMEM;
>>>>
>>>> @@ -804,7 +804,7 @@ void xnheap_destroy(struct xnheap *heap)
>>>>          nrheaps--;
>>>>          xnvfile_touch_tag(&vfile_tag);
>>>>          xnlock_put_irqrestore(&nklock, s);
>>>> -       kfree(heap->pagemap);
>>>> +       kvfree(heap->pagemap);
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(xnheap_destroy);
>>>>
>>>>
>>>
>>> Could you send a v2 of this patch with the more exhaustive description
>>> you had in v1?
>>>
>>> Thanks,
>>> Jan
>>>
>>> --
>>> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
>>> Corporate Competence Center Embedded Linux

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 13.08.20 04:26, Pintu Agarwal wrote:
> On Wed, 12 Aug 2020 at 21:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 11.08.20 13:44, Pintu Agarwal wrote:
>>> Oh sorry about this.
>>> I can send a new patch set with kvmalloc that can work on all kernel versions.
>>>
>>
>> Yes, please do so to avoid that things become to messy when trying to
>> create that manually.
>>
> 
> Thank you so much for your confirmation.
> Yes I am preparing a patch-set v2 and will send it soon.
> But is there a way to use both kvzalloc and kvmalloc depending on
> kernel version ?
> I see that checking using below is also not valid any more in higher
> kernel version:
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 9, 0)
> 

This pattern still works, also in 5.x.

> Also I see in some versions directly kmalloc is used (not kzalloc).
> Moreover, kvmalloc also may not be available in 3.x kernel version as well.
> So wondering if kvmalloc would be fine or not ?
> 
> Is covering 4.x kernel version should be enough ?
> 

Yes, 4.4 is our lower boundary for Xenomai 3.1.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[孙世龙 sunshilong]

>Can we just use vmalloc ?
>I think this is just an init code and it should not cause much impact
>with performance as in any case we are using GFP_KERNEL flag itself.
>Yes, I know the over-head about page-table and all with vmalloc

As what said in the last email, kvmalloc may be a better way than vmalloc:
kvmalloc(sizeof(struct xnheap_pgentry) * nrpages,
+                               GFP_KERNEL | __GFP_ZERO);

Besides, the kernel does implement kzvmalloc() in this way(which is
seen on https://elixir.bootlin.com/linux/latest/source/include/linux/mm.h#L759).
For your convenience, here is the related code snippet:
static inline void *kvzalloc(size_t size, gfp_t flags)
{
return kvmalloc(size, flags | __GFP_ZERO);
}

On Fri, Aug 14, 2020 at 1:49 AM Pintu Agarwal <pintu.ping@gmail.com> wrote:
>
> On Thu, 13 Aug 2020 at 16:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >
> > On 13.08.20 04:26, Pintu Agarwal wrote:
> > > On Wed, 12 Aug 2020 at 21:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > >>
> > >> On 11.08.20 13:44, Pintu Agarwal wrote:
> > >>> Oh sorry about this.
> > >>> I can send a new patch set with kvmalloc that can work on all kernel versions.
> > >>>
> > >>
> > >> Yes, please do so to avoid that things become to messy when trying to
> > >> create that manually.
> > >>
> > >
> > > Thank you so much for your confirmation.
> > > Yes I am preparing a patch-set v2 and will send it soon.
> > > But is there a way to use both kvzalloc and kvmalloc depending on
> > > kernel version ?
> > > I see that checking using below is also not valid any more in higher
> > > kernel version:
> > > #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 9, 0)
> > >
> >
> > This pattern still works, also in 5.x.
> >
> Yes, but it seems illegal to use in latest kernel version.
> The checkpatch.pl script in kernel catches these and that is why I see
> these pattern is no more used.
> scripts/checkpatch.pl:4157:# avoid LINUX_VERSION_CODE
> scripts/checkpatch.pl:4158:             if ($line =~ /\bLINUX_VERSION_CODE\b/) {
> scripts/checkpatch.pl:4159:                     WARN("LINUX_VERSION_CODE",
> scripts/checkpatch.pl:4160:
> "LINUX_VERSION_CODE should be avoided, code should be for the version
> to which it is merged\n" . $herecurr);
>
>
> Can we just use vmalloc ?
> I think this is just an init code and it should not cause much impact
> with performance as in any case we are using GFP_KERNEL flag itself.
> Yes, I know the over-head about page-table and all with vmalloc
>
> Anyways, I am exploring more what could be the best way.
>
> Thanks,
> Pintu

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 13.08.20 19:49, Pintu Agarwal wrote:
> On Thu, 13 Aug 2020 at 16:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 13.08.20 04:26, Pintu Agarwal wrote:
>>> On Wed, 12 Aug 2020 at 21:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>
>>>> On 11.08.20 13:44, Pintu Agarwal wrote:
>>>>> Oh sorry about this.
>>>>> I can send a new patch set with kvmalloc that can work on all kernel versions.
>>>>>
>>>>
>>>> Yes, please do so to avoid that things become to messy when trying to
>>>> create that manually.
>>>>
>>>
>>> Thank you so much for your confirmation.
>>> Yes I am preparing a patch-set v2 and will send it soon.
>>> But is there a way to use both kvzalloc and kvmalloc depending on
>>> kernel version ?
>>> I see that checking using below is also not valid any more in higher
>>> kernel version:
>>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 9, 0)
>>>
>>
>> This pattern still works, also in 5.x.
>>
> Yes, but it seems illegal to use in latest kernel version.
> The checkpatch.pl script in kernel catches these and that is why I see
> these pattern is no more used.
> scripts/checkpatch.pl:4157:# avoid LINUX_VERSION_CODE
> scripts/checkpatch.pl:4158:             if ($line =~ /\bLINUX_VERSION_CODE\b/) {
> scripts/checkpatch.pl:4159:                     WARN("LINUX_VERSION_CODE",
> scripts/checkpatch.pl:4160:
> "LINUX_VERSION_CODE should be avoided, code should be for the version
> to which it is merged\n" . $herecurr);
> 

It's illegal for in-tree code because that is implicitly in sync with 
the kernel version, always. But Xenomai is out-of-tree, thus needs to 
account for different versions it may be built against.

> 
> Can we just use vmalloc ?
> I think this is just an init code and it should not cause much impact
> with performance as in any case we are using GFP_KERNEL flag itself.
> Yes, I know the over-head about page-table and all with vmalloc

Yes, I also think we can go with vmalloc. A heap size smaller one page 
is rather unlikely, even more many of those heaps.

Jan

> 
> Anyways, I am exploring more what could be the best way.
> 
> Thanks,
> Pintu
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[孙世龙 sunshilong]

What about simulating the behavior of kvmalloc by vmalloc and kmalloc(
and simulating the behavior of kvfree by kfree and vfree)?
The said functions are available for Linux-4.x.
If there is something wrong, please let me know.

Something like this:
For simulating kvmalloc:
 gfp_t kmalloc_flags = GFP_KERNEL | __GFP_ZERO | __GFP_NORETRY;
if (size > PAGE_SIZE) {
kmalloc_flags |= __GFP_NOWARN;
}

heap->pagemap = kmalloc(sizeof(struct xnheap_pgentry) * nrpages,
                                kmalloc_flags);
if (NULL == heap->pagemap)
{
ptr = __vmalloc(size, GFP_KERNEL | __GFP_ZERO, PAGE_KERNEL);
}

For kvfree:
if (is_vmalloc_addr(addr))
vfree(addr);
else
kfree(addr);

Best Regards
Sunshilong


On Fri, Aug 14, 2020 at 3:37 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
>
> On Fri, 14 Aug 2020 at 12:36, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >
> > On 13.08.20 19:49, Pintu Agarwal wrote:
> > > On Thu, 13 Aug 2020 at 16:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > >>
> > >> On 13.08.20 04:26, Pintu Agarwal wrote:
> > >>> On Wed, 12 Aug 2020 at 21:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > >>>>
> > >>>> On 11.08.20 13:44, Pintu Agarwal wrote:
> > >>>>> Oh sorry about this.
> > >>>>> I can send a new patch set with kvmalloc that can work on all kernel versions.
> > >>>>>
> > >>>>
> > >>>> Yes, please do so to avoid that things become to messy when trying to
> > >>>> create that manually.
> > >>>>
> > >>>
> > >>> Thank you so much for your confirmation.
> > >>> Yes I am preparing a patch-set v2 and will send it soon.
> > >>> But is there a way to use both kvzalloc and kvmalloc depending on
> > >>> kernel version ?
> > >>> I see that checking using below is also not valid any more in higher
> > >>> kernel version:
> > >>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 9, 0)
> > >>>
> > >>
> > >> This pattern still works, also in 5.x.
> > >>
> > > Yes, but it seems illegal to use in latest kernel version.
> > > The checkpatch.pl script in kernel catches these and that is why I see
> > > these pattern is no more used.
> > > scripts/checkpatch.pl:4157:# avoid LINUX_VERSION_CODE
> > > scripts/checkpatch.pl:4158:             if ($line =~ /\bLINUX_VERSION_CODE\b/) {
> > > scripts/checkpatch.pl:4159:                     WARN("LINUX_VERSION_CODE",
> > > scripts/checkpatch.pl:4160:
> > > "LINUX_VERSION_CODE should be avoided, code should be for the version
> > > to which it is merged\n" . $herecurr);
> > >
> >
> > It's illegal for in-tree code because that is implicitly in sync with
> > the kernel version, always. But Xenomai is out-of-tree, thus needs to
> > account for different versions it may be built against.
> >
> > >
> > > Can we just use vmalloc ?
> > > I think this is just an init code and it should not cause much impact
> > > with performance as in any case we are using GFP_KERNEL flag itself.
> > > Yes, I know the over-head about page-table and all with vmalloc
> >
> > Yes, I also think we can go with vmalloc. A heap size smaller one page
> > is rather unlikely, even more many of those heaps.
> >
>
> Yes I think here vmalloc should be enough rather than maintaining
> different version for different kernel.
> And I found that even kvmalloc is not straight available <= kernel 4.9
> I can see its implementation only here, which seems to be specific to
> fs subsystem.
> fs/f2fs/f2fs.h:2757:static inline void *kvmalloc(size_t size, gfp_t flags)
>
> So, we don't have any other option.
>
> I will create patch with vmalloc itself then....
>
>
> Thanks,
> Pintu

Re: [PATCH] kernel:cobalt:heap: replace kzalloc with kvzalloc

[Jan Kiszka]

On 14.08.20 11:54, 孙世龙 sunshilong wrote:
> What about simulating the behavior of kvmalloc by vmalloc and kmalloc(
> and simulating the behavior of kvfree by kfree and vfree)?

I would agree but I do not see the use case for that finer-grained 
allocation. In that case, simpler is better.

Jan

> The said functions are available for Linux-4.x.
> If there is something wrong, please let me know.
> 
> Something like this:
> For simulating kvmalloc:
>   gfp_t kmalloc_flags = GFP_KERNEL | __GFP_ZERO | __GFP_NORETRY;
> if (size > PAGE_SIZE) {
> kmalloc_flags |= __GFP_NOWARN;
> }
> 
> heap->pagemap = kmalloc(sizeof(struct xnheap_pgentry) * nrpages,
>                                  kmalloc_flags);
> if (NULL == heap->pagemap)
> {
> ptr = __vmalloc(size, GFP_KERNEL | __GFP_ZERO, PAGE_KERNEL);
> }
> 
> For kvfree:
> if (is_vmalloc_addr(addr))
> vfree(addr);
> else
> kfree(addr);
> 
> Best Regards
> Sunshilong
> 
> 
> On Fri, Aug 14, 2020 at 3:37 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
>>
>> On Fri, 14 Aug 2020 at 12:36, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>
>>> On 13.08.20 19:49, Pintu Agarwal wrote:
>>>> On Thu, 13 Aug 2020 at 16:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>
>>>>> On 13.08.20 04:26, Pintu Agarwal wrote:
>>>>>> On Wed, 12 Aug 2020 at 21:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>
>>>>>>> On 11.08.20 13:44, Pintu Agarwal wrote:
>>>>>>>> Oh sorry about this.
>>>>>>>> I can send a new patch set with kvmalloc that can work on all kernel versions.
>>>>>>>>
>>>>>>>
>>>>>>> Yes, please do so to avoid that things become to messy when trying to
>>>>>>> create that manually.
>>>>>>>
>>>>>>
>>>>>> Thank you so much for your confirmation.
>>>>>> Yes I am preparing a patch-set v2 and will send it soon.
>>>>>> But is there a way to use both kvzalloc and kvmalloc depending on
>>>>>> kernel version ?
>>>>>> I see that checking using below is also not valid any more in higher
>>>>>> kernel version:
>>>>>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 9, 0)
>>>>>>
>>>>>
>>>>> This pattern still works, also in 5.x.
>>>>>
>>>> Yes, but it seems illegal to use in latest kernel version.
>>>> The checkpatch.pl script in kernel catches these and that is why I see
>>>> these pattern is no more used.
>>>> scripts/checkpatch.pl:4157:# avoid LINUX_VERSION_CODE
>>>> scripts/checkpatch.pl:4158:             if ($line =~ /\bLINUX_VERSION_CODE\b/) {
>>>> scripts/checkpatch.pl:4159:                     WARN("LINUX_VERSION_CODE",
>>>> scripts/checkpatch.pl:4160:
>>>> "LINUX_VERSION_CODE should be avoided, code should be for the version
>>>> to which it is merged\n" . $herecurr);
>>>>
>>>
>>> It's illegal for in-tree code because that is implicitly in sync with
>>> the kernel version, always. But Xenomai is out-of-tree, thus needs to
>>> account for different versions it may be built against.
>>>
>>>>
>>>> Can we just use vmalloc ?
>>>> I think this is just an init code and it should not cause much impact
>>>> with performance as in any case we are using GFP_KERNEL flag itself.
>>>> Yes, I know the over-head about page-table and all with vmalloc
>>>
>>> Yes, I also think we can go with vmalloc. A heap size smaller one page
>>> is rather unlikely, even more many of those heaps.
>>>
>>
>> Yes I think here vmalloc should be enough rather than maintaining
>> different version for different kernel.
>> And I found that even kvmalloc is not straight available <= kernel 4.9
>> I can see its implementation only here, which seems to be specific to
>> fs subsystem.
>> fs/f2fs/f2fs.h:2757:static inline void *kvmalloc(size_t size, gfp_t flags)
>>
>> So, we don't have any other option.
>>
>> I will create patch with vmalloc itself then....
>>
>>
>> Thanks,
>> Pintu

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[PATCH v2] kernel:cobalt:heap: replace kzalloc with vmalloc

[Pintu Kumar]

With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
based on their needs. For some application needs, this can grow as large
as 4MB that is, 2^10 order pages, which is unlikely to succeed with
kzalloc. Even the default (256KB) may fail on highly fragmented system.

Moreover, for this heap allocation, we don't need physical contiguous
memory. Thus vmalloc may be sufficient here.

Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
available in all kernel versions. Thus for backward compatibility we stick
to vmalloc at least till we support 4.x kernel.

Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
Signed-off-by: sunshilong <sunshilong369@gmail.com>
Tested-by: sunshilong <sunshilong369@gmail.com>
---
 kernel/cobalt/heap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
index d01a2e0..17b392b 100644
--- a/kernel/cobalt/heap.c
+++ b/kernel/cobalt/heap.c
@@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
 	xnlock_init(&heap->lock);
 
 	nrpages = size >> XNHEAP_PAGE_SHIFT;
-	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
-				GFP_KERNEL);
+	heap->pagemap = vmalloc(sizeof(struct xnheap_pgentry) * nrpages);
 	if (heap->pagemap == NULL)
 		return -ENOMEM;
 
@@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
 	nrheaps--;
 	xnvfile_touch_tag(&vfile_tag);
 	xnlock_put_irqrestore(&nklock, s);
-	kfree(heap->pagemap);
+	vfree(heap->pagemap);
 }
 EXPORT_SYMBOL_GPL(xnheap_destroy);
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2] kernel:cobalt:heap: replace kzalloc with vmalloc

[Jan Kiszka]

On 15.08.20 12:29, Pintu Kumar wrote:
> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
> based on their needs. For some application needs, this can grow as large
> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> kzalloc. Even the default (256KB) may fail on highly fragmented system.
> 
> Moreover, for this heap allocation, we don't need physical contiguous
> memory. Thus vmalloc may be sufficient here.
> 
> Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
> available in all kernel versions. Thus for backward compatibility we stick
> to vmalloc at least till we support 4.x kernel.
> 
> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> Signed-off-by: sunshilong <sunshilong369@gmail.com>
> Tested-by: sunshilong <sunshilong369@gmail.com>
> ---
>  kernel/cobalt/heap.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> index d01a2e0..17b392b 100644
> --- a/kernel/cobalt/heap.c
> +++ b/kernel/cobalt/heap.c
> @@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
>  	xnlock_init(&heap->lock);
>  
>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> -				GFP_KERNEL);
> +	heap->pagemap = vmalloc(sizeof(struct xnheap_pgentry) * nrpages);

Is vmalloc guaranteed to return zeroed pages? I don't think so.

Jan

>  	if (heap->pagemap == NULL)
>  		return -ENOMEM;
>  
> @@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
>  	nrheaps--;
>  	xnvfile_touch_tag(&vfile_tag);
>  	xnlock_put_irqrestore(&nklock, s);
> -	kfree(heap->pagemap);
> +	vfree(heap->pagemap);
>  }
>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>  
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] kernel:cobalt:heap: replace kzalloc with vmalloc

[Jan Kiszka]

On 18.08.20 07:44, Jan Kiszka wrote:
> On 15.08.20 12:29, Pintu Kumar wrote:
>> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
>> based on their needs. For some application needs, this can grow as large
>> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
>> kzalloc. Even the default (256KB) may fail on highly fragmented system.
>>
>> Moreover, for this heap allocation, we don't need physical contiguous
>> memory. Thus vmalloc may be sufficient here.
>>
>> Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
>> available in all kernel versions. Thus for backward compatibility we stick
>> to vmalloc at least till we support 4.x kernel.
>>
>> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
>> Signed-off-by: sunshilong <sunshilong369@gmail.com>
>> Tested-by: sunshilong <sunshilong369@gmail.com>
>> ---
>>  kernel/cobalt/heap.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
>> index d01a2e0..17b392b 100644
>> --- a/kernel/cobalt/heap.c
>> +++ b/kernel/cobalt/heap.c
>> @@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
>>  	xnlock_init(&heap->lock);
>>  
>>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
>> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
>> -				GFP_KERNEL);
>> +	heap->pagemap = vmalloc(sizeof(struct xnheap_pgentry) * nrpages);
> 
> Is vmalloc guaranteed to return zeroed pages? I don't think so.
> 

vzalloc is available for that, also in 4.4.

Jan

> 
>>  	if (heap->pagemap == NULL)
>>  		return -ENOMEM;
>>  
>> @@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
>>  	nrheaps--;
>>  	xnvfile_touch_tag(&vfile_tag);
>>  	xnlock_put_irqrestore(&nklock, s);
>> -	kfree(heap->pagemap);
>> +	vfree(heap->pagemap);
>>  }
>>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>>  
>>
> 


-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] kernel:cobalt:heap: replace kzalloc with vmalloc

[孙世龙 sunshilong]

Hi, Jan, Pintu

>Is vmalloc guaranteed to return zeroed pages? I don't think so.
Maybe, vzalloc is a choice.
I found it is supported on Linux-4.1、Linux-4.4、Linux-4.9 and Linux-4.19
(https://elixir.bootlin.com/linux/v4.1/source/mm/vmalloc.c#L1742).

On Tue, Aug 18, 2020 at 1:44 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 15.08.20 12:29, Pintu Kumar wrote:
> > With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
> > based on their needs. For some application needs, this can grow as large
> > as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> > kzalloc. Even the default (256KB) may fail on highly fragmented system.
> >
> > Moreover, for this heap allocation, we don't need physical contiguous
> > memory. Thus vmalloc may be sufficient here.
> >
> > Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
> > available in all kernel versions. Thus for backward compatibility we stick
> > to vmalloc at least till we support 4.x kernel.
> >
> > Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> > Signed-off-by: sunshilong <sunshilong369@gmail.com>
> > Tested-by: sunshilong <sunshilong369@gmail.com>
> > ---
> >  kernel/cobalt/heap.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> > index d01a2e0..17b392b 100644
> > --- a/kernel/cobalt/heap.c
> > +++ b/kernel/cobalt/heap.c
> > @@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
> >       xnlock_init(&heap->lock);
> >
> >       nrpages = size >> XNHEAP_PAGE_SHIFT;
> > -     heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> > -                             GFP_KERNEL);
> > +     heap->pagemap = vmalloc(sizeof(struct xnheap_pgentry) * nrpages);
>
> Is vmalloc guaranteed to return zeroed pages? I don't think so.
>
> Jan
>
> >       if (heap->pagemap == NULL)
> >               return -ENOMEM;
> >
> > @@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
> >       nrheaps--;
> >       xnvfile_touch_tag(&vfile_tag);
> >       xnlock_put_irqrestore(&nklock, s);
> > -     kfree(heap->pagemap);
> > +     vfree(heap->pagemap);
> >  }
> >  EXPORT_SYMBOL_GPL(xnheap_destroy);
> >
> >
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux

[PATCH v3] kernel:cobalt:heap: replace kzalloc with vzalloc

[Pintu Kumar]

With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
based on their needs. For some application needs, this can grow as large
as 4MB that is, 2^10 order pages, which is unlikely to succeed with
kzalloc. Even the default (256KB) may fail on highly fragmented system.

Moreover, for this heap allocation, we don't need physical contiguous
memory. Thus vmalloc/vzalloc may be sufficient here.

Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
available in all kernel versions. Thus for backward compatibility we stick
to vmalloc at least till we support 4.x kernel.

Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
Signed-off-by: sunshilong <sunshilong369@gmail.com>
Tested-by: sunshilong <sunshilong369@gmail.com>
---
 kernel/cobalt/heap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
index d01a2e0..5d14a47 100644
--- a/kernel/cobalt/heap.c
+++ b/kernel/cobalt/heap.c
@@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
 	xnlock_init(&heap->lock);
 
 	nrpages = size >> XNHEAP_PAGE_SHIFT;
-	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
-				GFP_KERNEL);
+	heap->pagemap = vzalloc(sizeof(struct xnheap_pgentry) * nrpages);
 	if (heap->pagemap == NULL)
 		return -ENOMEM;
 
@@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
 	nrheaps--;
 	xnvfile_touch_tag(&vfile_tag);
 	xnlock_put_irqrestore(&nklock, s);
-	kfree(heap->pagemap);
+	vfree(heap->pagemap);
 }
 EXPORT_SYMBOL_GPL(xnheap_destroy);
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3] kernel:cobalt:heap: replace kzalloc with vzalloc

[Jan Kiszka]

On 18.08.20 19:56, Pintu Kumar wrote:
> With CONFIG_XENO_OPT_PRIVATE_HEAPSZ, user can request any heap size
> based on their needs. For some application needs, this can grow as large
> as 4MB that is, 2^10 order pages, which is unlikely to succeed with
> kzalloc. Even the default (256KB) may fail on highly fragmented system.
> 
> Moreover, for this heap allocation, we don't need physical contiguous
> memory. Thus vmalloc/vzalloc may be sufficient here.
> 
> Note, we may also use kvzalloc/kvmalloc, but unfortunately these are not
> available in all kernel versions. Thus for backward compatibility we stick
> to vmalloc at least till we support 4.x kernel.
> 
> Signed-off-by: Pintu Kumar <pintu@codeaurora.org>
> Signed-off-by: sunshilong <sunshilong369@gmail.com>
> Tested-by: sunshilong <sunshilong369@gmail.com>
> ---
>  kernel/cobalt/heap.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/cobalt/heap.c b/kernel/cobalt/heap.c
> index d01a2e0..5d14a47 100644
> --- a/kernel/cobalt/heap.c
> +++ b/kernel/cobalt/heap.c
> @@ -749,8 +749,7 @@ int xnheap_init(struct xnheap *heap, void *membase, size_t size)
>  	xnlock_init(&heap->lock);
>  
>  	nrpages = size >> XNHEAP_PAGE_SHIFT;
> -	heap->pagemap = kzalloc(sizeof(struct xnheap_pgentry) * nrpages,
> -				GFP_KERNEL);
> +	heap->pagemap = vzalloc(sizeof(struct xnheap_pgentry) * nrpages);
>  	if (heap->pagemap == NULL)
>  		return -ENOMEM;
>  
> @@ -804,7 +803,7 @@ void xnheap_destroy(struct xnheap *heap)
>  	nrheaps--;
>  	xnvfile_touch_tag(&vfile_tag);
>  	xnlock_put_irqrestore(&nklock, s);
> -	kfree(heap->pagemap);
> +	vfree(heap->pagemap);
>  }
>  EXPORT_SYMBOL_GPL(xnheap_destroy);
>  
> 

Thanks, applied to next.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux