On Tue, Mar 2, 2021 at 5:34 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Tue 02-03-21 17:23:42, Muchun Song wrote: > > On Tue, Mar 2, 2021 at 4:44 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > On Tue 02-03-21 15:37:33, Muchun Song wrote: > > > > The alloc_thread_stack_node() cannot guarantee that allocated stack pages > > > > are in the same node when CONFIG_VMAP_STACK. Because we do not specify > > > > __GFP_THISNODE to __vmalloc_node_range(). Fix it by caling > > > > mod_lruvec_page_state() for each page one by one. > > > > > > What is the actual problem you are trying to address by this patch? > > > 991e7673859e has deliberately dropped the per page accounting. Can you > > > explain why that was incorrect? There surely is some imprecision > > > involved but does it matter and is it even observable? > > > > When I read the code of account_kernel_stack(), I see a comment that > > says "All stack pages are in the same node". I am confused about this. > > IIUC, there is no guarantee about this. Right? > > Yes there is no guarantee indeed. Please always make sure to describe > the underlying reasoning for the patch. Subject of this patch refers to > a fix without explaining the actual problem. If a change is motivated by > code reading then make it explicit. Also if you are refering to a > different commit by Fixes: tag then it would be really helpful to > explicitly mention why that commit is incorrect or cause a visible > problems. Got it. Thanks for your teaching. > > > Yeah, imprecision may > > not be a problem. But if this is what we did deliberately, I think that > > it is better to add a comment there. Thanks. > > Yes the comment is quite confusing. I suspect it meant to say > /* All stack pages are accounted to the same node */ > > -- > Michal Hocko > SUSE Labs
