The patch titled Subject: mm/gup: factor out duplicate code from four routines has been added to the -mm tree. Its filename is mm-gup-factor-out-duplicate-code-from-four-routines.patch This patch should soon appear at http://ozlabs.org/~akpm/mmots/broken-out/mm-gup-factor-out-duplicate-code-from-four-routines.patch and later at http://ozlabs.org/~akpm/mmotm/broken-out/mm-gup-factor-out-duplicate-code-from-four-routines.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** The -mm tree is included into linux-next and is updated there every 3-4 working days ------------------------------------------------------ From: John Hubbard <jhubbard@xxxxxxxxxx> Subject: mm/gup: factor out duplicate code from four routines Patch series "mm/gup: track dma-pinned pages: FOLL_PIN", v8. Overview: This is a prerequisite to solving the problem of proper interactions between file-backed pages, and [R]DMA activities, as discussed in [1], [2], [3], and in a remarkable number of email threads since about 2017. :) A new internal gup flag, FOLL_PIN is introduced, and thoroughly documented in the last patch's Documentation/vm/pin_user_pages.rst. I believe that this will provide a good starting point for doing the layout lease work that Ira Weiny has been working on. That's because these new wrapper functions provide a clean, constrained, systematically named set of functionality that, again, is required in order to even know if a page is "dma-pinned". In contrast to earlier approaches, the page tracking can be incrementally applied to the kernel call sites that, until now, have been simply calling get_user_pages() ("gup"). In other words, opt-in by changing from this: get_user_pages() (sets FOLL_GET) put_page() to this: pin_user_pages() (sets FOLL_PIN) unpin_user_page() Testing: I've done some overall kernel testing (LTP, and a few other goodies), and some directed testing to exercise some of the changes. And as you can see, gup_benchmark is enhanced to exercise this. Basically, I've been able to runtime test the core get_user_pages() and pin_user_pages() and related routines, but not so much on several of the call sites--but those are generally just a couple of lines changed, each. Not much of the kernel is actually using this, which on one hand reduces risk quite a lot. But on the other hand, testing coverage is low. So I'd love it if, in particular, the Infiniband and PowerPC folks could do a smoke test of this series for me. Runtime testing for the call sites so far is pretty light: * io_uring: Some directed tests from liburing exercise this, and they pass. * process_vm_access.c: A small directed test passes. * gup_benchmark: the enhanced version hits the new gup.c code, and passes. * infiniband: ran "ib_write_bw", which exercises the umem.c changes, but not the other changes. * VFIO: compiles (I'm vowing to set up a run time test soon, but it's not ready just yet) * powerpc: it compiles... * drm/via: compiles... * goldfish: compiles... * net/xdp: compiles... * media/v4l2: compiles... [1] Some slow progress on get_user_pages() (Apr 2, 2019): https://lwn.net/Articles/784574/ [2] DMA and get_user_pages() (LPC: Dec 12, 2018): https://lwn.net/Articles/774411/ [3] The trouble with get_user_pages() (Apr 30, 2018): https://lwn.net/Articles/753027/ This patch (of 26): 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. Factor out the common code into static functions, thus reducing the overall line count and the code's complexity. 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. Link: http://lkml.kernel.org/r/20191209225344.99740-2-jhubbard@xxxxxxxxxx Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx> Reviewed-by: Christoph Hellwig <hch@xxxxxx> Reviewed-by: Jérôme Glisse <jglisse@xxxxxxxxxx> Reviewed-by: Jan Kara <jack@xxxxxxx> Cc: Ira Weiny <ira.weiny@xxxxxxxxx> Cc: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxx> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> Cc: Dan Williams <dan.j.williams@xxxxxxxxx> Cc: Daniel Vetter <daniel@xxxxxxxx> Cc: Dave Chinner <david@xxxxxxxxxxxxx> Cc: David Airlie <airlied@xxxxxxxx> Cc: "David S . Miller" <davem@xxxxxxxxxxxxx> Cc: Jason Gunthorpe <jgg@xxxxxxxx> Cc: Jens Axboe <axboe@xxxxxxxxx> Cc: Jonathan Corbet <corbet@xxxxxxx> Cc: Magnus Karlsson <magnus.karlsson@xxxxxxxxx> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx> Cc: Michal Hocko <mhocko@xxxxxxxx> Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx> Cc: Paul Mackerras <paulus@xxxxxxxxx> Cc: Shuah Khan <shuah@xxxxxxxxxx> Cc: Vlastimil Babka <vbabka@xxxxxxx> Cc: Björn Töpel <bjorn.topel@xxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> Cc: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx> Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx> Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- mm/gup.c | 91 ++++++++++++++++++++--------------------------------- 1 file changed, 36 insertions(+), 55 deletions(-) --- a/mm/gup.c~mm-gup-factor-out-duplicate-code-from-four-routines +++ a/mm/gup.c @@ -1978,6 +1978,25 @@ static int __gup_device_huge_pud(pud_t p } #endif +static int record_subpages(struct page *page, unsigned long addr, + unsigned long end, struct page **pages) +{ + int nr; + + for (nr = 0; addr != end; addr += PAGE_SIZE) + pages[nr++] = page++; + + return nr; +} + +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) @@ -2007,32 +2026,20 @@ static int gup_hugepte(pte_t *ptep, unsi /* hugepages are never "special" */ VM_BUG_ON(!pfn_valid(pte_pfn(pte))); - refs = 0; head = pte_page(pte); - 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; } @@ -2079,28 +2086,19 @@ static int gup_huge_pmd(pmd_t orig, pmd_ return __gup_device_huge_pmd(orig, pmdp, addr, end, 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; } @@ -2120,28 +2118,19 @@ static int gup_huge_pud(pud_t orig, pud_ return __gup_device_huge_pud(orig, pudp, addr, end, 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; } @@ -2157,28 +2146,20 @@ static int gup_huge_pgd(pgd_t orig, pgd_ 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; } _ Patches currently in -mm which might be from jhubbard@xxxxxxxxxx are mm-gup-factor-out-duplicate-code-from-four-routines.patch mm-gup-move-try_get_compound_head-to-top-fix-minor-issues.patch mm-devmap-refactor-1-based-refcounting-for-zone_device-pages.patch goldish_pipe-rename-local-pin_user_pages-routine.patch mm-fix-get_user_pages_remotes-handling-of-foll_longterm.patch vfio-fix-foll_longterm-use-simplify-get_user_pages_remote-call.patch mm-gup-allow-foll_force-for-get_user_pages_fast.patch ib-umem-use-get_user_pages_fast-to-pin-dma-pages.patch mm-gup-introduce-pin_user_pages-and-foll_pin.patch goldish_pipe-convert-to-pin_user_pages-and-put_user_page.patch ib-corehwumem-set-foll_pin-via-pin_user_pages-fix-up-odp.patch mm-process_vm_access-set-foll_pin-via-pin_user_pages_remote.patch drm-via-set-foll_pin-via-pin_user_pages_fast.patch fs-io_uring-set-foll_pin-via-pin_user_pages.patch net-xdp-set-foll_pin-via-pin_user_pages.patch media-v4l2-core-set-pages-dirty-upon-releasing-dma-buffers.patch media-v4l2-core-pin_user_pages-foll_pin-and-put_user_page-conversion.patch vfio-mm-pin_user_pages-foll_pin-and-put_user_page-conversion.patch powerpc-book3s64-convert-to-pin_user_pages-and-put_user_page.patch powerpc-book3s64-convert-to-pin_user_pages-and-put_user_page-fix.patch mm-gup_benchmark-use-proper-foll_write-flags-instead-of-hard-coding-1.patch mm-tree-wide-rename-put_user_page-to-unpin_user_page.patch mm-gup-pass-flags-arg-to-__gup_device_-functions.patch mm-gup-track-foll_pin-pages.patch mm-gup_benchmark-support-pin_user_pages-and-related-calls.patch selftests-vm-run_vmtests-invoke-gup_benchmark-with-basic-foll_pin-coverage.patch