From cf0cac0a09341549dedabcfc2a66dcbc2eaaf2b9 Mon Sep 17 00:00:00 2001 From: Cesar Eduardo Barros Date: Tue, 11 Dec 2012 16:01:13 -0800 Subject: mm: refactor reinsert of swap_info in sys_swapoff() The block within sys_swapoff() which re-inserts the swap_info into the swap_list in case of failure of try_to_unuse() reads a few values outside the swap_lock. While this is safe at that point, it is subtle code. Simplify the code by moving the reading of these values to a separate function, refactoring it a bit so they are read from within the swap_lock. This is easier to understand, and matches better the way it worked before I unified the insertion of the swap_info from both sys_swapon and sys_swapoff. This change should make no functional difference. The only real change is moving the read of two or three structure fields to within the lock (frontswap_map_get() is nothing more than a read of p->frontswap_map). Signed-off-by: Cesar Eduardo Barros Cc: Konrad Rzeszutek Wilk Cc: Dan Magenheimer Cc: Mel Gorman Cc: Rik van Riel Cc: KAMEZAWA Hiroyuki Cc: Johannes Weiner Cc: Hugh Dickins Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/swapfile.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/mm/swapfile.c b/mm/swapfile.c index f91a25547ff..27a52b73157 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1443,13 +1443,12 @@ static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span) return generic_swapfile_activate(sis, swap_file, span); } -static void enable_swap_info(struct swap_info_struct *p, int prio, +static void _enable_swap_info(struct swap_info_struct *p, int prio, unsigned char *swap_map, unsigned long *frontswap_map) { int i, prev; - spin_lock(&swap_lock); if (prio >= 0) p->prio = prio; else @@ -1473,6 +1472,21 @@ static void enable_swap_info(struct swap_info_struct *p, int prio, else swap_info[prev]->next = p->type; frontswap_init(p->type); +} + +static void enable_swap_info(struct swap_info_struct *p, int prio, + unsigned char *swap_map, + unsigned long *frontswap_map) +{ + spin_lock(&swap_lock); + _enable_swap_info(p, prio, swap_map, frontswap_map); + spin_unlock(&swap_lock); +} + +static void reinsert_swap_info(struct swap_info_struct *p) +{ + spin_lock(&swap_lock); + _enable_swap_info(p, p->prio, p->swap_map, frontswap_map_get(p)); spin_unlock(&swap_lock); } @@ -1548,14 +1562,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) compare_swap_oom_score_adj(OOM_SCORE_ADJ_MAX, oom_score_adj); if (err) { - /* - * reading p->prio and p->swap_map outside the lock is - * safe here because only sys_swapon and sys_swapoff - * change them, and there can be no other sys_swapon or - * sys_swapoff for this swap_info_struct at this point. - */ /* re-insert swap space back into swap_list */ - enable_swap_info(p, p->prio, p->swap_map, frontswap_map_get(p)); + reinsert_swap_info(p); goto out_dput; } -- cgit v1.2.3