vmalloc: Removing incorrect logs when vmalloc failed
diff mbox series

Message ID 1594113232-32193-1-git-send-email-tiantao6@hisilicon.com
State New
Headers show
Series
  • vmalloc: Removing incorrect logs when vmalloc failed
Related show

Commit Message

Tian Tao July 7, 2020, 9:13 a.m. UTC
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 <tiantao6@hisilicon.com>
---
 mm/vmalloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Anshuman Khandual July 7, 2020, 9:48 a.m. UTC | #1
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 <tiantao6@hisilicon.com>
> ---
>  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);
>
Uladzislau Rezki July 7, 2020, 1:24 p.m. UTC | #2
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
tiantao (H) July 8, 2020, 12:51 a.m. UTC | #3
在 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
> 
> .
>
Uladzislau Rezki July 8, 2020, 1:48 p.m. UTC | #4
> > > 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
tiantao (H) July 9, 2020, 1:03 a.m. UTC | #5
在 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
> 
> .
>

Patch
diff mbox series

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);