--- v7
+++ v10
@@ -1,195 +1,63 @@
-There are four locations in gup.c that have a fair amount of code
-duplication. This means that changing one requires making the same
-changes in four places, not to mention reading the same code four
-times, and wondering if there are subtle differences.
+An upcoming patch uses try_get_compound_head() more widely,
+so move it to the top of gup.c.
-Factor out the common code into static functions, thus reducing the
-overall line count and the code's complexity.
+Also fix a tiny spelling error and a checkpatch.pl warning.
-Also, take the opportunity to slightly improve the efficiency of the
-error cases, by doing a mass subtraction of the refcount, surrounded
-by get_page()/put_page().
-
-Also, further simplify (slightly), by waiting until the the successful
-end of each routine, to increment *nr.
-
-Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
+Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
-Cc: Ira Weiny <ira.weiny@intel.com>
-Cc: Christoph Hellwig <hch@lst.de>
-Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
+Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
- mm/gup.c | 91 ++++++++++++++++++++++----------------------------------
- 1 file changed, 36 insertions(+), 55 deletions(-)
+ mm/gup.c | 29 +++++++++++++++--------------
+ 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
-index 85caf76b3012..f3c7d6625817 100644
+index f764432914c4..3ecce297a47f 100644
--- a/mm/gup.c
+++ b/mm/gup.c
-@@ -1969,6 +1969,25 @@ static int __gup_device_huge_pud(pud_t pud, pud_t *pudp, unsigned long addr,
- }
- #endif
+@@ -29,6 +29,21 @@ struct follow_page_context {
+ unsigned int page_mask;
+ };
-+static int __record_subpages(struct page *page, unsigned long addr,
-+ unsigned long end, struct page **pages)
++/*
++ * Return the compound head page with ref appropriately incremented,
++ * or NULL if that failed.
++ */
++static inline struct page *try_get_compound_head(struct page *page, int refs)
+{
-+ int nr;
++ struct page *head = compound_head(page);
+
-+ for (nr = 0; addr != end; addr += PAGE_SIZE)
-+ pages[nr++] = page++;
-+
-+ return nr;
++ if (WARN_ON_ONCE(page_ref_count(head) < 0))
++ return NULL;
++ if (unlikely(!page_cache_add_speculative(head, refs)))
++ return NULL;
++ return head;
+}
+
-+static void put_compound_head(struct page *page, int refs)
-+{
-+ /* Do a get_page() first, in case refs == page->_refcount */
-+ get_page(page);
-+ page_ref_sub(page, refs);
-+ put_page(page);
-+}
-+
- #ifdef CONFIG_ARCH_HAS_HUGEPD
- static unsigned long hugepte_addr_end(unsigned long addr, unsigned long end,
- unsigned long sz)
-@@ -1998,32 +2017,20 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
- /* hugepages are never "special" */
- VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
+ /**
+ * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
+ * @pages: array of pages to be maybe marked dirty, and definitely released.
+@@ -1807,20 +1822,6 @@ static void __maybe_unused undo_dev_pagemap(int *nr, int nr_start,
+ }
+ }
-- refs = 0;
- head = pte_page(pte);
+-/*
+- * Return the compund head page with ref appropriately incremented,
+- * or NULL if that failed.
+- */
+-static inline struct page *try_get_compound_head(struct page *page, int refs)
+-{
+- struct page *head = compound_head(page);
+- if (WARN_ON_ONCE(page_ref_count(head) < 0))
+- return NULL;
+- if (unlikely(!page_cache_add_speculative(head, refs)))
+- return NULL;
+- return head;
+-}
-
- page = head + ((addr & (sz-1)) >> PAGE_SHIFT);
-- do {
-- VM_BUG_ON(compound_head(page) != head);
-- pages[*nr] = page;
-- (*nr)++;
-- page++;
-- refs++;
-- } while (addr += PAGE_SIZE, addr != end);
-+ refs = __record_subpages(page, addr, end, pages + *nr);
-
- head = try_get_compound_head(head, refs);
-- if (!head) {
-- *nr -= refs;
-+ if (!head)
- return 0;
-- }
-
- if (unlikely(pte_val(pte) != pte_val(*ptep))) {
-- /* Could be optimized better */
-- *nr -= refs;
-- while (refs--)
-- put_page(head);
-+ put_compound_head(head, refs);
- return 0;
- }
-
-+ *nr += refs;
- SetPageReferenced(head);
- return 1;
- }
-@@ -2071,28 +2078,19 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
- pages, nr);
- }
-
-- refs = 0;
- page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
-- do {
-- pages[*nr] = page;
-- (*nr)++;
-- page++;
-- refs++;
-- } while (addr += PAGE_SIZE, addr != end);
-+ refs = __record_subpages(page, addr, end, pages + *nr);
-
- head = try_get_compound_head(pmd_page(orig), refs);
-- if (!head) {
-- *nr -= refs;
-+ if (!head)
- return 0;
-- }
-
- if (unlikely(pmd_val(orig) != pmd_val(*pmdp))) {
-- *nr -= refs;
-- while (refs--)
-- put_page(head);
-+ put_compound_head(head, refs);
- return 0;
- }
-
-+ *nr += refs;
- SetPageReferenced(head);
- return 1;
- }
-@@ -2114,28 +2112,19 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
- pages, nr);
- }
-
-- refs = 0;
- page = pud_page(orig) + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
-- do {
-- pages[*nr] = page;
-- (*nr)++;
-- page++;
-- refs++;
-- } while (addr += PAGE_SIZE, addr != end);
-+ refs = __record_subpages(page, addr, end, pages + *nr);
-
- head = try_get_compound_head(pud_page(orig), refs);
-- if (!head) {
-- *nr -= refs;
-+ if (!head)
- return 0;
-- }
-
- if (unlikely(pud_val(orig) != pud_val(*pudp))) {
-- *nr -= refs;
-- while (refs--)
-- put_page(head);
-+ put_compound_head(head, refs);
- return 0;
- }
-
-+ *nr += refs;
- SetPageReferenced(head);
- return 1;
- }
-@@ -2151,28 +2140,20 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
- return 0;
-
- BUILD_BUG_ON(pgd_devmap(orig));
-- refs = 0;
-+
- page = pgd_page(orig) + ((addr & ~PGDIR_MASK) >> PAGE_SHIFT);
-- do {
-- pages[*nr] = page;
-- (*nr)++;
-- page++;
-- refs++;
-- } while (addr += PAGE_SIZE, addr != end);
-+ refs = __record_subpages(page, addr, end, pages + *nr);
-
- head = try_get_compound_head(pgd_page(orig), refs);
-- if (!head) {
-- *nr -= refs;
-+ if (!head)
- return 0;
-- }
-
- if (unlikely(pgd_val(orig) != pgd_val(*pgdp))) {
-- *nr -= refs;
-- while (refs--)
-- put_page(head);
-+ put_compound_head(head, refs);
- return 0;
- }
-
-+ *nr += refs;
- SetPageReferenced(head);
- return 1;
- }
+ #ifdef CONFIG_ARCH_HAS_PTE_SPECIAL
+ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
+ unsigned int flags, struct page **pages, int *nr)
--
2.24.0