Messages in this thread | | | Subject | Re: [PATCH v2 2/2] mm: migrate: Allocate the node_demotion structure dynamically | From | Baolin Wang <> | Date | Thu, 11 Nov 2021 19:20:11 +0800 |
| |
On 2021/11/11 16:51, Huang, Ying wrote: > Baolin Wang <baolin.wang@linux.alibaba.com> writes: > >> For the worst case (MAX_NUMNODES=1024), the node_demotion structure can >> consume 32k bytes, which appears too large, so we can change to allocate >> node_demotion dynamically at initialization time. Meanwhile allocating >> the target demotion nodes array dynamically to select a suitable size >> according to the MAX_NUMNODES. >> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> --- >> mm/migrate.c | 38 +++++++++++++++++++++++++++++--------- >> 1 file changed, 29 insertions(+), 9 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 126e9e6..0145b38 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1152,10 +1152,11 @@ static int __unmap_and_move(struct page *page, struct page *newpage, >> #define DEFAULT_DEMOTION_TARGET_NODES 15 >> struct demotion_nodes { >> unsigned short nr; >> - short nodes[DEFAULT_DEMOTION_TARGET_NODES]; >> + short nodes[]; >> }; >> >> -static struct demotion_nodes node_demotion[MAX_NUMNODES] __read_mostly; >> +static struct demotion_nodes *node_demotion[MAX_NUMNODES] __read_mostly; >> +static unsigned short target_nodes_max; > > I think we can use something as below, > > #if MAX_NUMNODES < DEFAULT_DEMOTION_TARGET_NODES > #define DEMOTION_TARGET_NODES (MAX_NUMNODES - 1) > #else > #define DEMOTION_TARGET_NODES DEFAULT_DEMOTION_TARGET_NODES > #endif
Yes, looks better.
> > static struct demotion_nodes *node_demotion; > > Then we can allocate nr_node_ids * sizeof(struct demotion_nodes) for node_demotion.
Yeah, this is simple. The reason I want to declare the structure like "struct demotion_nodes *node_demotion[MAX_NUMNODES]" is that, we can validate the non-possible nodes which are invalid to demote memory, and in case the node_demotion[nid] is failed to be allocated which can be validated, though this is unlikely. However, I agree with you to keep things simple now and can be merged into patch 1. Will do in next version. Thanks.
| |