Commit 2d15eb31 authored by akpm@linux-foundation.org's avatar akpm@linux-foundation.org Committed by Linus Torvalds

mm/gup: add make_dirty arg to put_user_pages_dirty_lock()

[11~From: John Hubbard <jhubbard@nvidia.com>
Subject: mm/gup: add make_dirty arg to put_user_pages_dirty_lock()

Patch series "mm/gup: add make_dirty arg to put_user_pages_dirty_lock()",
v3.

There are about 50+ patches in my tree [2], and I'll be sending out the
remaining ones in a few more groups:

* The block/bio related changes (Jerome mostly wrote those, but I've had
  to move stuff around extensively, and add a little code)

* mm/ changes

* other subsystem patches

* an RFC that shows the current state of the tracking patch set.  That
  can only be applied after all call sites are converted, but it's good to
  get an early look at it.

This is part a tree-wide conversion, as described in fc1d8e7c ("mm:
introduce put_user_page*(), placeholder versions").

This patch (of 3):

Provide more capable variation of put_user_pages_dirty_lock(), and delete
put_user_pages_dirty().  This is based on the following:

1.  Lots of call sites become simpler if a bool is passed into
   put_user_page*(), instead of making the call site choose which
   put_user_page*() variant to call.

2.  Christoph Hellwig's observation that set_page_dirty_lock() is
   usually correct, and set_page_dirty() is usually a bug, or at least
   questionable, within a put_user_page*() calling chain.

This leads to the following API choices:

    * put_user_pages_dirty_lock(page, npages, make_dirty)

    * There is no put_user_pages_dirty(). You have to
      hand code that, in the rare case that it's
      required.

[jhubbard@nvidia.com: remove unused variable in siw_free_plist()]
  Link: http://lkml.kernel.org/r/20190729074306.10368-1-jhubbard@nvidia.com
Link: http://lkml.kernel.org/r/20190724044537.10458-2-jhubbard@nvidia.comSigned-off-by: default avatarJohn Hubbard <jhubbard@nvidia.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 1ba6fc9a
...@@ -54,10 +54,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d ...@@ -54,10 +54,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) { for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) {
page = sg_page_iter_page(&sg_iter); page = sg_page_iter_page(&sg_iter);
if (umem->writable && dirty) put_user_pages_dirty_lock(&page, 1, umem->writable && dirty);
put_user_pages_dirty_lock(&page, 1);
else
put_user_page(page);
} }
sg_free_table(&umem->sg_head); sg_free_table(&umem->sg_head);
......
...@@ -118,10 +118,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np ...@@ -118,10 +118,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
void hfi1_release_user_pages(struct mm_struct *mm, struct page **p, void hfi1_release_user_pages(struct mm_struct *mm, struct page **p,
size_t npages, bool dirty) size_t npages, bool dirty)
{ {
if (dirty) put_user_pages_dirty_lock(p, npages, dirty);
put_user_pages_dirty_lock(p, npages);
else
put_user_pages(p, npages);
if (mm) { /* during close after signal, mm can be NULL */ if (mm) { /* during close after signal, mm can be NULL */
atomic64_sub(npages, &mm->pinned_vm); atomic64_sub(npages, &mm->pinned_vm);
......
...@@ -40,10 +40,7 @@ ...@@ -40,10 +40,7 @@
static void __qib_release_user_pages(struct page **p, size_t num_pages, static void __qib_release_user_pages(struct page **p, size_t num_pages,
int dirty) int dirty)
{ {
if (dirty) put_user_pages_dirty_lock(p, num_pages, dirty);
put_user_pages_dirty_lock(p, num_pages);
else
put_user_pages(p, num_pages);
} }
/** /**
......
...@@ -75,10 +75,7 @@ static void usnic_uiom_put_pages(struct list_head *chunk_list, int dirty) ...@@ -75,10 +75,7 @@ static void usnic_uiom_put_pages(struct list_head *chunk_list, int dirty)
for_each_sg(chunk->page_list, sg, chunk->nents, i) { for_each_sg(chunk->page_list, sg, chunk->nents, i) {
page = sg_page(sg); page = sg_page(sg);
pa = sg_phys(sg); pa = sg_phys(sg);
if (dirty) put_user_pages_dirty_lock(&page, 1, dirty);
put_user_pages_dirty_lock(&page, 1);
else
put_user_page(page);
usnic_dbg("pa: %pa\n", &pa); usnic_dbg("pa: %pa\n", &pa);
} }
kfree(chunk); kfree(chunk);
......
...@@ -63,15 +63,7 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int stag_index) ...@@ -63,15 +63,7 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int stag_index)
static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages, static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages,
bool dirty) bool dirty)
{ {
struct page **p = chunk->plist; put_user_pages_dirty_lock(chunk->plist, num_pages, dirty);
while (num_pages--) {
if (!PageDirty(*p) && dirty)
put_user_pages_dirty_lock(p, 1);
else
put_user_page(*p);
p++;
}
} }
void siw_umem_release(struct siw_umem *umem, bool dirty) void siw_umem_release(struct siw_umem *umem, bool dirty)
......
...@@ -1075,8 +1075,9 @@ static inline void put_user_page(struct page *page) ...@@ -1075,8 +1075,9 @@ static inline void put_user_page(struct page *page)
put_page(page); put_page(page);
} }
void put_user_pages_dirty(struct page **pages, unsigned long npages); void put_user_pages_dirty_lock(struct page **pages, unsigned long npages,
void put_user_pages_dirty_lock(struct page **pages, unsigned long npages); bool make_dirty);
void put_user_pages(struct page **pages, unsigned long npages); void put_user_pages(struct page **pages, unsigned long npages);
#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
......
...@@ -29,85 +29,70 @@ struct follow_page_context { ...@@ -29,85 +29,70 @@ struct follow_page_context {
unsigned int page_mask; unsigned int page_mask;
}; };
typedef int (*set_dirty_func_t)(struct page *page);
static void __put_user_pages_dirty(struct page **pages,
unsigned long npages,
set_dirty_func_t sdf)
{
unsigned long index;
for (index = 0; index < npages; index++) {
struct page *page = compound_head(pages[index]);
/*
* Checking PageDirty at this point may race with
* clear_page_dirty_for_io(), but that's OK. Two key cases:
*
* 1) This code sees the page as already dirty, so it skips
* the call to sdf(). That could happen because
* clear_page_dirty_for_io() called page_mkclean(),
* followed by set_page_dirty(). However, now the page is
* going to get written back, which meets the original
* intention of setting it dirty, so all is well:
* clear_page_dirty_for_io() goes on to call
* TestClearPageDirty(), and write the page back.
*
* 2) This code sees the page as clean, so it calls sdf().
* The page stays dirty, despite being written back, so it
* gets written back again in the next writeback cycle.
* This is harmless.
*/
if (!PageDirty(page))
sdf(page);
put_user_page(page);
}
}
/** /**
* put_user_pages_dirty() - release and dirty an array of gup-pinned pages * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
* @pages: array of pages to be marked dirty and released. * @pages: array of pages to be maybe marked dirty, and definitely released.
* @npages: number of pages in the @pages array. * @npages: number of pages in the @pages array.
* @make_dirty: whether to mark the pages dirty
* *
* "gup-pinned page" refers to a page that has had one of the get_user_pages() * "gup-pinned page" refers to a page that has had one of the get_user_pages()
* variants called on that page. * variants called on that page.
* *
* For each page in the @pages array, make that page (or its head page, if a * For each page in the @pages array, make that page (or its head page, if a
* compound page) dirty, if it was previously listed as clean. Then, release * compound page) dirty, if @make_dirty is true, and if the page was previously
* the page using put_user_page(). * listed as clean. In any case, releases all pages using put_user_page(),
* possibly via put_user_pages(), for the non-dirty case.
* *
* Please see the put_user_page() documentation for details. * Please see the put_user_page() documentation for details.
* *
* set_page_dirty(), which does not lock the page, is used here. * set_page_dirty_lock() is used internally. If instead, set_page_dirty() is
* Therefore, it is the caller's responsibility to ensure that this is * required, then the caller should a) verify that this is really correct,
* safe. If not, then put_user_pages_dirty_lock() should be called instead. * because _lock() is usually required, and b) hand code it:
* set_page_dirty_lock(), put_user_page().
* *
*/ */
void put_user_pages_dirty(struct page **pages, unsigned long npages) void put_user_pages_dirty_lock(struct page **pages, unsigned long npages,
bool make_dirty)
{ {
__put_user_pages_dirty(pages, npages, set_page_dirty); unsigned long index;
}
EXPORT_SYMBOL(put_user_pages_dirty);
/** /*
* put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages * TODO: this can be optimized for huge pages: if a series of pages is
* @pages: array of pages to be marked dirty and released. * physically contiguous and part of the same compound page, then a
* @npages: number of pages in the @pages array. * single operation to the head page should suffice.
*
* For each page in the @pages array, make that page (or its head page, if a
* compound page) dirty, if it was previously listed as clean. Then, release
* the page using put_user_page().
*
* Please see the put_user_page() documentation for details.
*
* This is just like put_user_pages_dirty(), except that it invokes
* set_page_dirty_lock(), instead of set_page_dirty().
*
*/ */
void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
{ if (!make_dirty) {
__put_user_pages_dirty(pages, npages, set_page_dirty_lock); put_user_pages(pages, npages);
return;
}
for (index = 0; index < npages; index++) {
struct page *page = compound_head(pages[index]);
/*
* Checking PageDirty at this point may race with
* clear_page_dirty_for_io(), but that's OK. Two key
* cases:
*
* 1) This code sees the page as already dirty, so it
* skips the call to set_page_dirty(). That could happen
* because clear_page_dirty_for_io() called
* page_mkclean(), followed by set_page_dirty().
* However, now the page is going to get written back,
* which meets the original intention of setting it
* dirty, so all is well: clear_page_dirty_for_io() goes
* on to call TestClearPageDirty(), and write the page
* back.
*
* 2) This code sees the page as clean, so it calls
* set_page_dirty(). The page stays dirty, despite being
* written back, so it gets written back again in the
* next writeback cycle. This is harmless.
*/
if (!PageDirty(page))
set_page_dirty_lock(page);
put_user_page(page);
}
} }
EXPORT_SYMBOL(put_user_pages_dirty_lock); EXPORT_SYMBOL(put_user_pages_dirty_lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment