mm/swap: use nr_node_ids for avail_lists in swap_info_struct
Subsystems (2)
PATCH 1/1 mm/swap: use nr_node_ids for avail_lists in swap_info_struct
Since commit a2468cc9bfdf ("swap: choose swap device according to numa node"), avail_lists field of swap_info_struct is changed to an array with MAX_NUMNODES elements. This made swap_info_struct size increased to 40KiB and needs an order-4 page to hold it.
This is not optimal in that:
1 Most systems have way less than MAX_NUMNODES(1024) nodes so it
is a waste of memory;
2 It could cause swapon failure if the swap device is swapped on
after system has been running for a while, due to no order-4 page is available as pointed out by Vasily Averin.
Solve the above two issues by using nr_node_ids(which is the actual possible node number the running system has) for avail_lists instead of MAX_NUMNODES.
nr_node_ids is unknown at compile time so can't be directly used when declaring this array. What I did here is to declare avail_lists as zero element array and allocate space for it when allocating space for swap_info_struct. The reason why keep using array but not pointer is plist_for_each_entry needs the field to be part of the struct, so pointer will not work.
This patch is on top of Vasily Averin's fix commit. I think the use of kvzalloc for swap_info_struct is still needed in case nr_node_ids is really big on some systems.
--- include/linux/swap.h | 11 ++++++++++- mm/swapfile.c | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-)
include/linux/swap.h +10 -1
@@ -233,7 +233,6 @@ struct swap_info_struct {unsigned long flags;/* SWP_USED etc: see above */ signed short prio;/* swap priority of this type */ struct plist_node list;/* entry in swap_active_head */ -struct plist_node avail_lists[MAX_NUMNODES ];/* entry in swap_avail_heads */ signed char type;/* strange name for an index */ unsigned int max;/* extent of the swap_map */ unsigned char *swap_map;/* vmalloc'ed array of usage counts */
@@ -274,6 +273,16 @@ struct swap_info_struct {*/ struct work_struct discard_work;/* discard worker */ struct swap_cluster_list discard_clusters;/* discard clusters list */ +struct plist_node avail_lists[0 ];/* +* entries in swap_avail_heads, one +* entry per node. +* Must be last as the number of the +* array is nr_node_ids, which is not +* a fixed value so have to allocate +* dynamically. +* And it has to be an array so that +* plist_for_each_* can work. +*/ };#ifdef CONFIG_64BIT
mm/swapfile.c +2 -1
@@ -2812,8 +2812,9 @@ static struct swap_info_struct *alloc_swap_info(void)struct swap_info_struct *p;unsigned int type;int i; +int size =sizeof (*p) + nr_node_ids *sizeof (struct plist_node ); - p =kvzalloc (sizeof (*p),GFP_KERNEL ); + p =kvzalloc (size,GFP_KERNEL );if (!p)return ERR_PTR (-ENOMEM );
Signature · 1 line
2.17.2
Quoted 23 lines · Aaron Lu
Since commit
a2468cc9bfdf("swap: choose swap device according to numa node"),avail_listsfield ofswap_info_structis changed to an array withMAX_NUMNODESelements. This madeswap_info_structsize increased to 40KiB and needs an order-4 page to hold it.This is not optimal in that:
1 Most systems have way less thanMAX_NUMNODES(1024)nodes so it
is a waste of memory;
2 It could cause swapon failure if the swap device is swapped on
after system has been running for a while, due to no order-4 page is available as pointed out by Vasily Averin.Solve the above two issues by using
nr_node_ids(which is the actual possible node number the running system has)foravail_listsinstead ofMAX_NUMNODES.
nr_node_idsis unknown at compile time so can't be directly used when declaring this array. What I did here is to declareavail_listsas zero element array and allocate space for it when allocating space forswap_info_struct. The reason why keep using array but not pointer isplist_for_each_entryneeds the field to be part of the struct, so pointer will not work.
Quoted 3 lines
This patch is on top of Vasily Averin's fix commit. I think the use of kvzalloc for
swap_info_structis still needed in casenr_node_idsis really big on some systems.
I haven't seen any system with a huge nr_node_ids but using kvzalloc is not harmfull.
Quoted 4 lines
Cc Vasily Averin <[email protected]>Cc Michal Hocko <[email protected]>Cc Huang Ying <[email protected]>Signed-off-by Aaron Lu <[email protected]>
Quoted 52 lines
--- include/linux/swap.h | 11 ++++++++++- mm/swapfile.c | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-)include/linux/swap.h +10
-1@@ -233,7 +233,6 @@ struct swap_info_struct {unsigned long flags;/* SWP_USED etc: see above */ signed short prio;/* swap priority of this type */ struct plist_node list;/* entry in swap_active_head */ -struct plist_node avail_lists[MAX_NUMNODES ];/* entry in swap_avail_heads */ signed char type;/* strange name for an index */ unsigned int max;/* extent of the swap_map */ unsigned char *swap_map;/* vmalloc'ed array of usage counts */ @@ -274,6 +273,16 @@ struct swap_info_struct {*/ struct work_struct discard_work;/* discard worker */ struct swap_cluster_list discard_clusters;/* discard clusters list */ +struct plist_node avail_lists[0 ];/* +* entries in swap_avail_heads, one +* entry per node. +* Must be last as the number of the +* array is nr_node_ids, which is not +* a fixed value so have to allocate +* dynamically. +* And it has to be an array so that +* plist_for_each_* can work. +*/ };#ifdef CONFIG_64BIT mm/swapfile.c +2
-1@@ -2812,8 +2812,9 @@ static struct swap_info_struct *alloc_swap_info(void)struct swap_info_struct *p;unsigned int type;int i; +int size =sizeof (*p) + nr_node_ids *sizeof (struct plist_node ); - p =kvzalloc (sizeof (*p),GFP_KERNEL ); + p =kvzalloc (size,GFP_KERNEL );if (!p)return ERR_PTR (-ENOMEM );Signature · 1 line
2.17.2
Signature · 2 lines
Michal Hocko SUSE Labs