2020-07-07 09:18:04

by Tian Tao

[permalink] [raw]
Subject: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed

It is not possible to increase size with vmalloc=<size> in arm64
architecture and it will mislead.however vmalloc return failure
is a rare occurrence in 'many architectures including arm64'.

Signed-off-by: Tian Tao <[email protected]>
---
mm/vmalloc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 89e83d3..c7b5a9a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1238,8 +1238,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
}

if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
- pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
- size);
+ pr_warn("vmap allocation for size %lu failed\n", size);

kmem_cache_free(vmap_area_cachep, va);
return ERR_PTR(-EBUSY);
--
2.7.4


2020-07-07 09:52:06

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed



On 07/07/2020 02:43 PM, Tian Tao wrote:
> It is not possible to increase size with vmalloc=<size> in arm64
> architecture and it will mislead.however vmalloc return failure
> is a rare occurrence in 'many architectures including arm64'.

But there is a chance that vmalloc() might work on architectures
that support 'vmalloc=' command line i.e after a change and this
information here might be helpful in those cases.

>
> Signed-off-by: Tian Tao <[email protected]>
> ---
> mm/vmalloc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 89e83d3..c7b5a9a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1238,8 +1238,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> }
>
> if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
> - pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
> - size);
> + pr_warn("vmap allocation for size %lu failed\n", size);
>
> kmem_cache_free(vmap_area_cachep, va);
> return ERR_PTR(-EBUSY);
>

2020-07-07 13:28:34

by Uladzislau Rezki

[permalink] [raw]
Subject: Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed

On Tue, Jul 07, 2020 at 03:18:54PM +0530, Anshuman Khandual wrote:
>
>
> On 07/07/2020 02:43 PM, Tian Tao wrote:
> > It is not possible to increase size with vmalloc=<size> in arm64
> > architecture and it will mislead.however vmalloc return failure
> > is a rare occurrence in 'many architectures including arm64'.
>
> But there is a chance that vmalloc() might work on architectures
> that support 'vmalloc=' command line i.e after a change and this
> information here might be helpful in those cases.
>
Agree. At least i see a few users of it:

<snip>
urezki@pc638:~/data/coding/linux-next.git$ grep -rn early_param ./arch/ | grep vmalloc
./arch/arm/mm/mmu.c:1152:early_param("vmalloc", early_vmalloc);
./arch/unicore32/mm/mmu.c:276:early_param("vmalloc", early_vmalloc);
./arch/x86/mm/pgtable_32.c:86:early_param("vmalloc", parse_vmalloc);
urezki@pc638:~/data/coding/linux-next.git$
<snip>

Thanks!

--
Vlad Rezki

2020-07-08 01:28:05

by tiantao (H)

[permalink] [raw]
Subject: Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed



?? 2020/7/7 21:24, Uladzislau Rezki д??:
> On Tue, Jul 07, 2020 at 03:18:54PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 07/07/2020 02:43 PM, Tian Tao wrote:
>>> It is not possible to increase size with vmalloc=<size> in arm64
>>> architecture and it will mislead.however vmalloc return failure
>>> is a rare occurrence in 'many architectures including arm64'.
>>
>> But there is a chance that vmalloc() might work on architectures
>> that support 'vmalloc=' command line i.e after a change and this
>> information here might be helpful in those cases.
>>
> Agree. At least i see a few users of it:
>
> <snip>
> urezki@pc638:~/data/coding/linux-next.git$ grep -rn early_param ./arch/ | grep vmalloc
> ./arch/arm/mm/mmu.c:1152:early_param("vmalloc", early_vmalloc);
> ./arch/unicore32/mm/mmu.c:276:early_param("vmalloc", early_vmalloc);
> ./arch/x86/mm/pgtable_32.c:86:early_param("vmalloc", parse_vmalloc);
> urezki@pc638:~/data/coding/linux-next.git$
> <snip>
>
I'm actually having this problem with the arm64 architecture at centos
7.6 and pagesize is 64K.
I followed the prompts and added vmalloc=<size> to the command to
increase the size of the vmalloc.and found out it's not worked.
It took me some time to find out that this doesn't work for the arm64
architecture, so this log is misleading on arm64.
I think it's better not to be prompted than to be prompted incorrectly.
I'm sure there will be others with similar problems.
So I'd like to solve this problem this time, Please help me with your
suggestions.
If I change the PATCH to the following, will you accept it?

if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
+#ifdef CONFIG_ARM64 && CONFIG_XXX
+ pr_warn("vmap allocation for size %lu failed\n", size);
+#else
pr_warn("vmap allocation for size %lu failed: use
vmalloc=<size> to increase size\n",
size);
+#endif

> Thanks!
>
> --
> Vlad Rezki
>
> .
>

2020-07-08 13:51:40

by Uladzislau Rezki

[permalink] [raw]
Subject: Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed

> > > On 07/07/2020 02:43 PM, Tian Tao wrote:
> > > > It is not possible to increase size with vmalloc=<size> in arm64
> > > > architecture and it will mislead.however vmalloc return failure
> > > > is a rare occurrence in 'many architectures including arm64'.
> > >
> > > But there is a chance that vmalloc() might work on architectures
> > > that support 'vmalloc=' command line i.e after a change and this
> > > information here might be helpful in those cases.
> > >
> > Agree. At least i see a few users of it:
> >
> > <snip>
> > urezki@pc638:~/data/coding/linux-next.git$ grep -rn early_param ./arch/ | grep vmalloc
> > ./arch/arm/mm/mmu.c:1152:early_param("vmalloc", early_vmalloc);
> > ./arch/unicore32/mm/mmu.c:276:early_param("vmalloc", early_vmalloc);
> > ./arch/x86/mm/pgtable_32.c:86:early_param("vmalloc", parse_vmalloc);
> > urezki@pc638:~/data/coding/linux-next.git$
> > <snip>
> >
> I'm actually having this problem with the arm64 architecture at centos 7.6
> and pagesize is 64K.
> I followed the prompts and added vmalloc=<size> to the command to increase
> the size of the vmalloc.and found out it's not worked.
> It took me some time to find out that this doesn't work for the arm64
> architecture, so this log is misleading on arm64.
>
Agree, it can take time to understand some code or logic behind of it.
So in that case having good documentation or comments always help.

> I think it's better not to be prompted than to be prompted incorrectly.
> I'm sure there will be others with similar problems.
> So I'd like to solve this problem this time, Please help me with your
> suggestions.
> If I change the PATCH to the following, will you accept it?
>
Actually it is not up to me to decide what to take or not. Andrew Morton
is the key person here :) I can just review or make some comments same
as others.

> if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
> +#ifdef CONFIG_ARM64 && CONFIG_XXX
> + pr_warn("vmap allocation for size %lu failed\n", size);
> +#else
> pr_warn("vmap allocation for size %lu failed: use vmalloc=<size>
> to increase size\n",
> size);
> +#endif
>
I do not have a strong opinion here, but counting arches seems odd.
Maybe modify the string with following message:

<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 1f46c3b86f9f..0aa26bc128d7 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1202,7 +1202,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
}

if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
- pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
+ pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size, if your ARCH supports it\n",
size);

kmem_cache_free(vmap_area_cachep, va);
<snip>

--
Vlad Rezki

2020-07-09 01:04:16

by tiantao (H)

[permalink] [raw]
Subject: Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed



?? 2020/7/8 21:48, Uladzislau Rezki д??:
>>>> On 07/07/2020 02:43 PM, Tian Tao wrote:
>>>>> It is not possible to increase size with vmalloc=<size> in arm64
>>>>> architecture and it will mislead.however vmalloc return failure
>>>>> is a rare occurrence in 'many architectures including arm64'.
>>>>
>>>> But there is a chance that vmalloc() might work on architectures
>>>> that support 'vmalloc=' command line i.e after a change and this
>>>> information here might be helpful in those cases.
>>>>
>>> Agree. At least i see a few users of it:
>>>
>>> <snip>
>>> urezki@pc638:~/data/coding/linux-next.git$ grep -rn early_param ./arch/ | grep vmalloc
>>> ./arch/arm/mm/mmu.c:1152:early_param("vmalloc", early_vmalloc);
>>> ./arch/unicore32/mm/mmu.c:276:early_param("vmalloc", early_vmalloc);
>>> ./arch/x86/mm/pgtable_32.c:86:early_param("vmalloc", parse_vmalloc);
>>> urezki@pc638:~/data/coding/linux-next.git$
>>> <snip>
>>>
>> I'm actually having this problem with the arm64 architecture at centos 7.6
>> and pagesize is 64K.
>> I followed the prompts and added vmalloc=<size> to the command to increase
>> the size of the vmalloc.and found out it's not worked.
>> It took me some time to find out that this doesn't work for the arm64
>> architecture, so this log is misleading on arm64.
>>
> Agree, it can take time to understand some code or logic behind of it.
> So in that case having good documentation or comments always help.
>
>> I think it's better not to be prompted than to be prompted incorrectly.
>> I'm sure there will be others with similar problems.
>> So I'd like to solve this problem this time, Please help me with your
>> suggestions.
>> If I change the PATCH to the following, will you accept it?
>>
> Actually it is not up to me to decide what to take or not. Andrew Morton
> is the key person here :) I can just review or make some comments same
> as others.
>
>> if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
>> +#ifdef CONFIG_ARM64 && CONFIG_XXX
>> + pr_warn("vmap allocation for size %lu failed\n", size);
>> +#else
>> pr_warn("vmap allocation for size %lu failed: use vmalloc=<size>
>> to increase size\n",
>> size);
>> +#endif
>>
> I do not have a strong opinion here, but counting arches seems odd.
> Maybe modify the string with following message:
>
> <snip>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 1f46c3b86f9f..0aa26bc128d7 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1202,7 +1202,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> }
>
> if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
> - pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
> + pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size, if your ARCH supports it\n",
> size);
>
> kmem_cache_free(vmap_area_cachep, va);
> <snip>
>
Thanks, that's a good suggestion, I'll send v2 as you suggested!
> --
> Vlad Rezki
>
> .
>