On Mon, Dec 21, 2020 at 12:15 PM Vijayanand Jitta <vji...@codeaurora.org> wrote: > > > > On 12/18/2020 2:10 PM, Vijayanand Jitta wrote: > > > > > > On 12/17/2020 4:24 PM, Alexander Potapenko wrote: > >>>> Can you provide an example of a use case in which the user wants to > >>>> use the stack depot of a smaller size without disabling it completely, > >>>> and that size cannot be configured statically? > >>>> As far as I understand, for the page owner example you gave it's > >>>> sufficient to provide a switch that can disable the stack depot if > >>>> page_owner=off. > >>>> > >>> There are two use cases here, > >>> > >>> 1. We don't want to consume memory when page_owner=off ,boolean flag > >>> would work here. > >>> > >>> 2. We would want to enable page_owner on low ram devices but we don't > >>> want stack depot to consume 8 MB of memory, so for this case we would > >>> need a configurable stack_hash_size so that we can still use page_owner > >>> with lower memory consumption. > >>> > >>> So, a configurable stack_hash_size would work for both these use cases, > >>> we can set it to '0' for first case and set the required size for the > >>> second case. > >> > >> Will a combined solution with a boolean boot-time flag and a static > >> CONFIG_STACKDEPOT_HASH_SIZE work for these cases? > >> I suppose low-memory devices have a separate kernel config anyway? > >> > > > > Yes, the combined solution will also work but i think having a single > > run time config is simpler instead of having two things to configure. > > > > To add to it we started of with a CONFIG first, after the comments from > Minchan (https://lkml.org/lkml/2020/11/3/2121) we decided to switch to > run time param. > > Quoting Minchan's comments below: > > " > 1. When we don't use page_owner, we don't want to waste any memory for > stackdepot hash array. > 2. When we use page_owner, we want to have reasonable stackdeport hash array > > With this configuration, it couldn't meet since we always need to > reserve a reasonable size for the array. > Can't we make the hash size as a kernel parameter? > With it, we could use it like this. > > 1. page_owner=off, stackdepot_stack_hash=0 -> no more wasted memory > when we don't use page_owner > 2. page_owner=on, stackdepot_stack_hash=8M -> reasonable hash size > when we use page_owner. > "
Minchan, what do you think about making the hash size itself a static parameter, while letting the user disable stackdepot completely at runtime? As noted before, I am concerned that moving a low-level configuration bit (which essentially means "save 8Mb - (1 << stackdepot_stack_hash) of static memory") to the boot parameters will be unused by most admins and may actually trick them into thinking they reduce the overall stackdepot memory consumption noticeably. I also suppose device vendors may prefer setting a fixed (maybe non-default) hash size for low-memory devices rather than letting the admins increase it. Alex PS. Sorry for being late to the party, I should have probably spoken up in November, when you've been discussing the first version of this patch. > Thanks, > Vijay > >> My concern is that exposing yet another knob to users won't really > >> solve their problems, because the hash size alone doesn't give enough > >> control over stackdepot memory footprint (we also have stack_slabs, > >> which may get way bigger than 8Mb). > >> > > > > True, stack_slabs can consume more memory but they consume most only > > when stack depot is used as they are allocated in stack_depot_save path. > > when stack depot is not used they consume 8192 * sizeof(void) bytes at > > max. So nothing much we can do here since static allocation is not much > > and memory consumption depends up on stack depot usage, unlike > > stack_hash_table where 8mb is preallocated. > > > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a > member of Code Aurora Forum, hosted by The Linux Foundation -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg