Messages in this thread | ![/](/images/icornerl.gif) | | From | Yang Shi <> | Date | Fri, 4 Nov 2022 13:40:46 -0700 | Subject | Re: [v2 PATCH 1/2] mm: khugepaged: allow page allocation fallback to eligible nodes |
| |
On Fri, Nov 4, 2022 at 12:55 PM Michal Hocko <mhocko@suse.com> wrote: > > On Fri 04-11-22 10:37:39, Yang Shi wrote: > > On Fri, Nov 4, 2022 at 1:32 AM Michal Hocko <mhocko@suse.com> wrote: > > > > > > On Thu 03-11-22 14:36:40, Yang Shi wrote: > > > [...] > > > > So use nodemask to record the nodes which have the same hit record, the > > > > hugepage allocation could fallback to those nodes. And remove > > > > __GFP_THISNODE since it does disallow fallback. And if nodemask is > > > > empty (no node is set), it means there is one single node has the most > > > > hist record, the nodemask approach actually behaves like __GFP_THISNODE. > > > > > > > > Reported-by: syzbot+0044b22d177870ee974f@syzkaller.appspotmail.com > > > > Suggested-by: Zach O'Keefe <zokeefe@google.com> > > > > Suggested-by: Michal Hocko <mhocko@suse.com> > > > > Signed-off-by: Yang Shi <shy828301@gmail.com> > > > > --- > > > > mm/khugepaged.c | 32 ++++++++++++++------------------ > > > > 1 file changed, 14 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > > > index ea0d186bc9d4..572ce7dbf4b0 100644 > > > > --- a/mm/khugepaged.c > > > > +++ b/mm/khugepaged.c > > > > @@ -97,8 +97,8 @@ struct collapse_control { > > > > /* Num pages scanned per node */ > > > > u32 node_load[MAX_NUMNODES]; > > > > > > > > - /* Last target selected in hpage_collapse_find_target_node() */ > > > > - int last_target_node; > > > > + /* nodemask for allocation fallback */ > > > > + nodemask_t alloc_nmask; > > > > > > This will eat another 1k on the stack on most configurations > > > (NODE_SHIFT=10). Along with 4k of node_load this is quite a lot even > > > on shallow call chains like madvise resp. khugepaged. I would just > > > add a follow up patch which changes both node_load and alloc_nmask to > > > dynamically allocated objects. > > > > The collapse_control is allocated by kmalloc dynamically for > > MADV_COLLAPSE path, and defined as a global variable for khugepaged > > (khugepaged_collapse_control). So it is not on stack. > > Dang, I must have been blind because I _think_ I have seen it as a local > stack defined. Maybe I just implicitly put that to the same bucket as > othe $foo_control (e.g. scan_control, oom_control etc) which leave on the > stack usually. Sorry about the confusion. Sorry for the noise.
It doesn't matter. It was not put on the stack due to its size when Zach was adding MADV_COLLAPSE.
> > Acked-by: Michal Hocko <mhocko@suse.com>
Thanks.
> -- > Michal Hocko > SUSE Labs
| ![\](/images/icornerr.gif) |