On 1/5/2021 2:54 PM, Vijayanand Jitta wrote: > > > On 1/5/2021 4:42 AM, Andrew Morton wrote: >> On Wed, 30 Dec 2020 18:15:30 +0530 vji...@codeaurora.org wrote: >> >>> Use STACK_HASH_ORDER_SHIFT to configure STACK_HASH_SIZE. >>> >>> Aim is to have configurable value for STACK_HASH_SIZE, >>> so depend on use case one can configure it. >>> >>> One example is of Page Owner, default value of >>> STACK_HASH_SIZE lead stack depot to consume 8MB of static memory. >>> Making it configurable and use lower value helps to enable features like >>> CONFIG_PAGE_OWNER without any significant overhead. >> >> Questions regarding the stackdepot code. >> >> - stack_table_tmp[] is __initdata. So after initmem is released, >> that "consume 8MB of static memory" should no longer be true. But >> iirc, not all architectures actually release __initdata memory. Does >> your architecture do this? >> > Thanks for review comments, I wasn't aware that __initdata is > architecture dependent, I was assuming that __initdata always frees > memory and yes the architecture which i am using (arm64) does free > __inidata. > >> - Stackdepot copies stack_table_tmp[] into vmalloced memory during >> initcalls. Why? Why not simply make stack_table_tmp[] no longer >> __initdata and use that memory for all time? >> >> Presumably because in the stack_depot_disable==true case, we >> release stack_table_tmp[] memory, don't vmalloc for a copy of it, and >> save a bunch of memory? If so, this assumes that the __initdata >> memory is freed. >> > > Yes, that correct. assumption here is __initidata will free memory if > stack_depot_disable=true is set. > >> - Why is that hash table so large? Is it appropriately sized? >> > > I think the large size of hash table is justified since the users of > stack depot like kasan, page owner etc store a very large number of stacks. > >> - SMP is up and running during init_stackdepot(), I think? If so, is >> that huge memcpy smp-safe? Can other CPUs be modifying >> stack_table_tmp[] while the memcpy is in flight? >> >> >> > Yes, parallel access could be possible. I will add a locking mechanism > inplace. > > Thanks, > Vijay > I have updated the patch avoiding __initdata as per suggestion and the copy from tmp , can you please review v5. https://lore.kernel.org/patchwork/patch/1367306/ Thanks, Vijay -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
