Messages in this thread | | | Subject | Re: [PATCH] vmalloc: Removing incorrect logs when vmalloc failed | From | "tiantao (H)" <> | Date | Thu, 9 Jul 2020 09:03:13 +0800 |
| |
在 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 > > . >
| |