diff for duplicates of <20160503101153.GA7241@gmail.com> diff --git a/a/1.txt b/N1/1.txt index 8289132..858c07d 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -124,4 +124,4 @@ get_user_pages(vaddr, nrpages, 1, 0|1, pages, NULL|vmas); Cheers, -Jerome +Jérôme diff --git a/a/2.txt b/N1/2.txt index 8b13789..bebc22d 100644 --- a/a/2.txt +++ b/N1/2.txt @@ -1 +1,95 @@ +>From 9ded2a5da75a5e736fb36a2c4e2511d9516ecc37 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com> +Date: Tue, 3 May 2016 11:53:24 +0200 +Subject: [PATCH] mm/numa/thp: fix assumptions of + migrate_misplaced_transhuge_page() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit +Fix assumptions in migrate_misplaced_transhuge_page() which is only +call by do_huge_pmd_numa_page() itself only call by __handle_mm_fault() +for pmd with PROT_NONE. This means that if the pmd stays the same +then there can be no concurrent get_user_pages / get_user_pages_fast +(GUP/GUP_fast). More over because migrate_misplaced_transhuge_page() +only do something is page is map once then there can be no GUP from +a different process. Finaly, holding the pmd lock assure us that no +other part of the kernel will take an extre reference on the page. + +In the end this means that the failure code path should never be +taken unless something is horribly wrong, so convert it to BUG_ON(). + +Signed-off-by: Jérôme Glisse <jglisse@redhat.com> +Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com +Cc: Mel Gorman <mgorman@suse.de> +Cc: Hugh Dickins <hughd@google.com> +Cc: Andrea Arcangeli <aarcange@redhat.com> +--- + mm/migrate.c | 31 +++++++++++++++++++++---------- + 1 file changed, 21 insertions(+), 10 deletions(-) + +diff --git a/mm/migrate.c b/mm/migrate.c +index 6c822a7..6315aac 100644 +--- a/mm/migrate.c ++++ b/mm/migrate.c +@@ -1757,6 +1757,14 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm, + pmd_t orig_entry; + + /* ++ * What we do here is only valid if pmd_protnone(entry) is true and it ++ * is map in only one vma numamigrate_isolate_page() takes care of that ++ * check. ++ */ ++ if (!pmd_protnone(entry)) ++ goto out_unlock; ++ ++ /* + * Rate-limit the amount of data that is being migrated to a node. + * Optimal placement is no good if the memory bus is saturated and + * all the time is being spent migrating! +@@ -1797,7 +1805,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm, + mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end); + ptl = pmd_lock(mm, pmd); + if (unlikely(!pmd_same(*pmd, entry) || page_count(page) != 2)) { +-fail_putback: + spin_unlock(ptl); + mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end); + +@@ -1819,7 +1826,12 @@ fail_putback: + goto out_unlock; + } + +- orig_entry = *pmd; ++ /* ++ * We are holding the lock so no one can set a new pmd and original pmd ++ * is PROT_NONE thus no one can get_user_pages or get_user_pages_fast ++ * (GUP or GUP_fast) from this point on we can not fail. ++ */ ++ orig_entry = entry; + entry = mk_pmd(new_page, vma->vm_page_prot); + entry = pmd_mkhuge(entry); + entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma); +@@ -1837,14 +1849,13 @@ fail_putback: + set_pmd_at(mm, mmun_start, pmd, entry); + update_mmu_cache_pmd(vma, address, &entry); + +- if (page_count(page) != 2) { +- set_pmd_at(mm, mmun_start, pmd, orig_entry); +- flush_pmd_tlb_range(vma, mmun_start, mmun_end); +- mmu_notifier_invalidate_range(mm, mmun_start, mmun_end); +- update_mmu_cache_pmd(vma, address, &entry); +- page_remove_rmap(new_page, true); +- goto fail_putback; +- } ++ /* As said above no one can get reference on the old page nor through ++ * get_user_pages or get_user_pages_fast (GUP/GUP_fast) or through ++ * any other means. To get reference on huge page you need to hold ++ * pmd_lock and we are already holding that lock here and the page ++ * is only mapped once. ++ */ ++ BUG_ON(page_count(page) != 2); + + mlock_migrate_page(new_page, page); + page_remove_rmap(page, true); +-- +2.1.0 diff --git a/a/content_digest b/N1/content_digest index 67ed253..7d1bb1e 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -141,9 +141,104 @@ "\n" "\n" "Cheers,\n" - Jerome + "J\303\251r\303\264me" "\01:2\0" "fn\00001-mm-numa-thp-fix-assumptions-of-migrate_misplaced_tra.patch\0" "b\0" + ">From 9ded2a5da75a5e736fb36a2c4e2511d9516ecc37 Mon Sep 17 00:00:00 2001\n" + "From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>\n" + "Date: Tue, 3 May 2016 11:53:24 +0200\n" + "Subject: [PATCH] mm/numa/thp: fix assumptions of\n" + " migrate_misplaced_transhuge_page()\n" + "MIME-Version: 1.0\n" + "Content-Type: text/plain; charset=UTF-8\n" + "Content-Transfer-Encoding: 8bit\n" + "\n" + "Fix assumptions in migrate_misplaced_transhuge_page() which is only\n" + "call by do_huge_pmd_numa_page() itself only call by __handle_mm_fault()\n" + "for pmd with PROT_NONE. This means that if the pmd stays the same\n" + "then there can be no concurrent get_user_pages / get_user_pages_fast\n" + "(GUP/GUP_fast). More over because migrate_misplaced_transhuge_page()\n" + "only do something is page is map once then there can be no GUP from\n" + "a different process. Finaly, holding the pmd lock assure us that no\n" + "other part of the kernel will take an extre reference on the page.\n" + "\n" + "In the end this means that the failure code path should never be\n" + "taken unless something is horribly wrong, so convert it to BUG_ON().\n" + "\n" + "Signed-off-by: J\303\251r\303\264me Glisse <jglisse@redhat.com>\n" + "Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com\n" + "Cc: Mel Gorman <mgorman@suse.de>\n" + "Cc: Hugh Dickins <hughd@google.com>\n" + "Cc: Andrea Arcangeli <aarcange@redhat.com>\n" + "---\n" + " mm/migrate.c | 31 +++++++++++++++++++++----------\n" + " 1 file changed, 21 insertions(+), 10 deletions(-)\n" + "\n" + "diff --git a/mm/migrate.c b/mm/migrate.c\n" + "index 6c822a7..6315aac 100644\n" + "--- a/mm/migrate.c\n" + "+++ b/mm/migrate.c\n" + "@@ -1757,6 +1757,14 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,\n" + " \tpmd_t orig_entry;\n" + " \n" + " \t/*\n" + "+\t * What we do here is only valid if pmd_protnone(entry) is true and it\n" + "+\t * is map in only one vma numamigrate_isolate_page() takes care of that\n" + "+\t * check.\n" + "+\t */\n" + "+\tif (!pmd_protnone(entry))\n" + "+\t\tgoto out_unlock;\n" + "+\n" + "+\t/*\n" + " \t * Rate-limit the amount of data that is being migrated to a node.\n" + " \t * Optimal placement is no good if the memory bus is saturated and\n" + " \t * all the time is being spent migrating!\n" + "@@ -1797,7 +1805,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,\n" + " \tmmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);\n" + " \tptl = pmd_lock(mm, pmd);\n" + " \tif (unlikely(!pmd_same(*pmd, entry) || page_count(page) != 2)) {\n" + "-fail_putback:\n" + " \t\tspin_unlock(ptl);\n" + " \t\tmmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);\n" + " \n" + "@@ -1819,7 +1826,12 @@ fail_putback:\n" + " \t\tgoto out_unlock;\n" + " \t}\n" + " \n" + "-\torig_entry = *pmd;\n" + "+\t/*\n" + "+\t * We are holding the lock so no one can set a new pmd and original pmd\n" + "+\t * is PROT_NONE thus no one can get_user_pages or get_user_pages_fast\n" + "+\t * (GUP or GUP_fast) from this point on we can not fail.\n" + "+\t */\n" + "+\torig_entry = entry;\n" + " \tentry = mk_pmd(new_page, vma->vm_page_prot);\n" + " \tentry = pmd_mkhuge(entry);\n" + " \tentry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);\n" + "@@ -1837,14 +1849,13 @@ fail_putback:\n" + " \tset_pmd_at(mm, mmun_start, pmd, entry);\n" + " \tupdate_mmu_cache_pmd(vma, address, &entry);\n" + " \n" + "-\tif (page_count(page) != 2) {\n" + "-\t\tset_pmd_at(mm, mmun_start, pmd, orig_entry);\n" + "-\t\tflush_pmd_tlb_range(vma, mmun_start, mmun_end);\n" + "-\t\tmmu_notifier_invalidate_range(mm, mmun_start, mmun_end);\n" + "-\t\tupdate_mmu_cache_pmd(vma, address, &entry);\n" + "-\t\tpage_remove_rmap(new_page, true);\n" + "-\t\tgoto fail_putback;\n" + "-\t}\n" + "+\t/* As said above no one can get reference on the old page nor through\n" + "+\t * get_user_pages or get_user_pages_fast (GUP/GUP_fast) or through\n" + "+\t * any other means. To get reference on huge page you need to hold\n" + "+\t * pmd_lock and we are already holding that lock here and the page\n" + "+\t * is only mapped once.\n" + "+\t */\n" + "+\tBUG_ON(page_count(page) != 2);\n" + " \n" + " \tmlock_migrate_page(new_page, page);\n" + " \tpage_remove_rmap(page, true);\n" + "-- \n" + 2.1.0 -85675e6afa589e2fe9446e58f04e8cd90c4450f43c229ede7539504ff3802c3d +408e56731ee300211f40281695a9a20b371d1c0eb0982535c9f7396d0b2a46e1
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.