ksm: fix conflict between mmput and scan_get_next_rmap_item
A concurrency issue about KSM in the function scan_get_next_rmap_item. task A (ksmd): |task B (the mm's task): | mm = slot->mm; | down_read(&mm->mmap_sem); | | ... | | spin_lock(&ksm_mmlist_lock); | | ksm_scan.mm_slot go to the next slot; | | spin_unlock(&ksm_mmlist_lock); | |mmput() -> | ksm_exit(): | |spin_lock(&ksm_mmlist_lock); |if (mm_slot && ksm_scan.mm_slot != mm_slot) { | if (!mm_slot->rmap_list) { | easy_to_free = 1; | ... | |if (easy_to_free) { | mmdrop(mm); | ... | |So this mm_struct may be freed in the mmput(). | up_read(&mm->mmap_sem); | As we can see above, the ksmd thread may access a mm_struct that already been freed to the kmem_cache. Suppose a fork will get this mm_struct from the kmem_cache, the ksmd thread then call up_read(&mm->mmap_sem), will cause mmap_sem.count to become -1. As suggested by Andrea Arcangeli, unmerge_and_remove_all_rmap_items has the same SMP race condition, so fix it too. My prev fix in function scan_get_next_rmap_item will introduce a different SMP race condition, so just invert the up_read/spin_unlock order as Andrea Arcangeli said. Link: http://lkml.kernel.org/r/1462708815-31301-1-git-send-email-zhouchengming1@huawei.com Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com> Suggested-by: Andrea Arcangeli <aarcange@redhat.com> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com> Cc: Hugh Dickins <hughd@google.com> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Geliang Tang <geliangtang@163.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Hanjun Guo <guohanjun@huawei.com> Cc: Ding Tianhong <dingtianhong@huawei.com> Cc: Li Bin <huawei.libin@huawei.com> Cc: Zhen Lei <thunder.leizhen@huawei.com> Cc: Xishi Qiu <qiuxishi@huawei.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
c25a1e0671
commit
7496fea9a6
15
mm/ksm.c
15
mm/ksm.c
@ -783,6 +783,7 @@ static int unmerge_and_remove_all_rmap_items(void)
|
||||
}
|
||||
|
||||
remove_trailing_rmap_items(mm_slot, &mm_slot->rmap_list);
|
||||
up_read(&mm->mmap_sem);
|
||||
|
||||
spin_lock(&ksm_mmlist_lock);
|
||||
ksm_scan.mm_slot = list_entry(mm_slot->mm_list.next,
|
||||
@ -794,12 +795,9 @@ static int unmerge_and_remove_all_rmap_items(void)
|
||||
|
||||
free_mm_slot(mm_slot);
|
||||
clear_bit(MMF_VM_MERGEABLE, &mm->flags);
|
||||
up_read(&mm->mmap_sem);
|
||||
mmdrop(mm);
|
||||
} else {
|
||||
} else
|
||||
spin_unlock(&ksm_mmlist_lock);
|
||||
up_read(&mm->mmap_sem);
|
||||
}
|
||||
}
|
||||
|
||||
/* Clean up stable nodes, but don't worry if some are still busy */
|
||||
@ -1663,8 +1661,15 @@ next_mm:
|
||||
up_read(&mm->mmap_sem);
|
||||
mmdrop(mm);
|
||||
} else {
|
||||
spin_unlock(&ksm_mmlist_lock);
|
||||
up_read(&mm->mmap_sem);
|
||||
/*
|
||||
* up_read(&mm->mmap_sem) first because after
|
||||
* spin_unlock(&ksm_mmlist_lock) run, the "mm" may
|
||||
* already have been freed under us by __ksm_exit()
|
||||
* because the "mm_slot" is still hashed and
|
||||
* ksm_scan.mm_slot doesn't point to it anymore.
|
||||
*/
|
||||
spin_unlock(&ksm_mmlist_lock);
|
||||
}
|
||||
|
||||
/* Repeat until we've completed scanning the whole list */
|
||||
|
Loading…
Reference in New Issue
Block a user