From: john.hubbard@gmail.com To: Andrew Morton <akpm@linux-foundation.org> Cc: "Alexander Viro" <viro@zeniv.linux.org.uk>, "Björn Töpel" <bjorn.topel@intel.com>, "Boaz Harrosh" <boaz@plexistor.com>, "Christoph Hellwig" <hch@lst.de>, "Daniel Vetter" <daniel@ffwll.ch>, "Dan Williams" <dan.j.williams@intel.com>, "Dave Chinner" <david@fromorbit.com>, "David Airlie" <airlied@linux.ie>, "David S . Miller" <davem@davemloft.net>, "Ilya Dryomov" <idryomov@gmail.com>, "Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>, "Jens Axboe" <axboe@kernel.dk>, "Jérôme Glisse" <jglisse@redhat.com>, "Johannes Thumshirn" <jthumshirn@suse.de>, "Magnus Karlsson" <magnus.karlsson@intel.com>, "Matthew Wilcox" <willy@infradead.org>, "Miklos Szeredi" <miklos@szeredi.hu>, "Ming Lei" <ming.lei@redhat.com>, "Sage Weil" <sage@redhat.com>, "Santosh Shilimkar" <santosh.shilimkar@oracle.com>, "Yan Zheng" <zyan@redhat.com>, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mm@kvack.org, linux-rdma@vger.kernel.org, bpf@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, "John Hubbard" <jhubbard@nvidia.com> Subject: [PATCH 1/3] mm/gup: introduce __put_user_pages() Date: Mon, 22 Jul 2019 15:34:13 -0700 [thread overview] Message-ID: <20190722223415.13269-2-jhubbard@nvidia.com> (raw) In-Reply-To: <20190722223415.13269-1-jhubbard@nvidia.com> From: John Hubbard <jhubbard@nvidia.com> Add a more capable variation of put_user_pages() to the API set, and call it from the simple ones. The new __put_user_pages() takes an enum that handles the various combinations of needing to call set_page_dirty() or set_page_dirty_lock(), before calling put_user_page(). Cc: Matthew Wilcox <willy@infradead.org> Cc: Jan Kara <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- include/linux/mm.h | 58 ++++++++++++++++++- mm/gup.c | 137 ++++++++++++++++++++++----------------------- 2 files changed, 124 insertions(+), 71 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 0334ca97c584..7218585681b2 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1057,8 +1057,62 @@ static inline void put_user_page(struct 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); +enum pup_flags_t { + PUP_FLAGS_CLEAN = 0, + PUP_FLAGS_DIRTY = 1, + PUP_FLAGS_LOCK = 2, + PUP_FLAGS_DIRTY_LOCK = 3, +}; + +void __put_user_pages(struct page **pages, unsigned long npages, + enum pup_flags_t flags); + +/** + * put_user_pages_dirty() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * "gup-pinned page" refers to a page that has had one of the get_user_pages() + * variants called on that page. + * + * 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. + * + * set_page_dirty(), which does not lock the page, is used here. + * Therefore, it is the caller's responsibility to ensure that this is + * safe. If not, then put_user_pages_dirty_lock() should be called instead. + * + */ +static inline void put_user_pages_dirty(struct page **pages, + unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY); +} + +/** + * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * 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(). + * + */ +static inline void put_user_pages_dirty_lock(struct page **pages, + unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY_LOCK); +} + void put_user_pages(struct page **pages, unsigned long npages); #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) diff --git a/mm/gup.c b/mm/gup.c index 98f13ab37bac..6831ef064d76 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -29,87 +29,86 @@ struct follow_page_context { 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() - release an array of gup-pinned pages. * @pages: array of pages to be marked dirty and released. * @npages: number of pages in the @pages array. + * @flags: additional hints, to be applied to each page: * - * "gup-pinned page" refers to a page that has had one of the get_user_pages() - * variants called on that page. + * PUP_FLAGS_CLEAN: no additional steps required. (Consider calling + * put_user_pages() directly, instead.) * - * 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(). + * PUP_FLAGS_DIRTY: Call set_page_dirty() on the page (if not already + * dirty). * - * Please see the put_user_page() documentation for details. + * PUP_FLAGS_LOCK: meaningless by itself, but included in order to show + * the numeric relationship between the flags. * - * set_page_dirty(), which does not lock the page, is used here. - * Therefore, it is the caller's responsibility to ensure that this is - * safe. If not, then put_user_pages_dirty_lock() should be called instead. + * PUP_FLAGS_DIRTY_LOCK: Call set_page_dirty_lock() on the page (if not + * already dirty). * + * For each page in the @pages array, release the page using put_user_page(). */ -void put_user_pages_dirty(struct page **pages, unsigned long npages) +void __put_user_pages(struct page **pages, unsigned long npages, + enum pup_flags_t flags) { - __put_user_pages_dirty(pages, npages, set_page_dirty); -} -EXPORT_SYMBOL(put_user_pages_dirty); + unsigned long index; -/** - * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages - * @pages: array of pages to be marked dirty and released. - * @npages: number of pages in the @pages array. - * - * 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) -{ - __put_user_pages_dirty(pages, npages, set_page_dirty_lock); + /* + * TODO: this can be optimized for huge pages: if a series of pages is + * physically contiguous and part of the same compound page, then a + * single operation to the head page should suffice. + */ + + for (index = 0; index < npages; index++) { + struct page *page = compound_head(pages[index]); + + switch (flags) { + case PUP_FLAGS_CLEAN: + break; + + case PUP_FLAGS_DIRTY: + /* + * 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(page); + break; + + case PUP_FLAGS_LOCK: + VM_WARN_ON_ONCE(flags == PUP_FLAGS_LOCK); + /* + * Shouldn't happen, but treat it as _DIRTY_LOCK if + * it does: fall through. + */ + + case PUP_FLAGS_DIRTY_LOCK: + /* Same comments as for PUP_FLAGS_DIRTY apply here. */ + if (!PageDirty(page)) + set_page_dirty_lock(page); + break; + }; + put_user_page(page); + } } -EXPORT_SYMBOL(put_user_pages_dirty_lock); +EXPORT_SYMBOL(__put_user_pages); /** * put_user_pages() - release an array of gup-pinned pages. -- 2.22.0
WARNING: multiple messages have this Message-ID
From: john.hubbard@gmail.com To: Andrew Morton <akpm@linux-foundation.org> Cc: "Alexander Viro" <viro@zeniv.linux.org.uk>, "Björn Töpel" <bjorn.topel@intel.com>, "Boaz Harrosh" <boaz@plexistor.com>, "Christoph Hellwig" <hch@lst.de>, "Daniel Vetter" <daniel@ffwll.ch>, "Dan Williams" <dan.j.williams@intel.com>, "Dave Chinner" <david@fromorbit.com>, "David Airlie" <airlied@linux.ie>, "David S . Miller" <davem@davemloft.net>, "Ilya Dryomov" <idryomov@gmail.com>, "Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>, "Jens Axboe" <axboe@kernel.dk>, "Jérôme Glisse" <jglisse@redhat.com>, "Johannes Thumshirn" <jthumshirn@suse.de>, "Magnus Karlsson" <magnus.karlsson@intel.com>, "Matthew Wilcox" <willy@infradead.org>, "Miklos Szeredi" <miklos@szeredi.hu>, "Ming Lei" <ming.lei@redhat.com>, "Sage Weil" <sage@redhat.com> Subject: [PATCH 1/3] mm/gup: introduce __put_user_pages() Date: Mon, 22 Jul 2019 15:34:13 -0700 [thread overview] Message-ID: <20190722223415.13269-2-jhubbard@nvidia.com> (raw) In-Reply-To: <20190722223415.13269-1-jhubbard@nvidia.com> From: John Hubbard <jhubbard@nvidia.com> Add a more capable variation of put_user_pages() to the API set, and call it from the simple ones. The new __put_user_pages() takes an enum that handles the various combinations of needing to call set_page_dirty() or set_page_dirty_lock(), before calling put_user_page(). Cc: Matthew Wilcox <willy@infradead.org> Cc: Jan Kara <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- include/linux/mm.h | 58 ++++++++++++++++++- mm/gup.c | 137 ++++++++++++++++++++++----------------------- 2 files changed, 124 insertions(+), 71 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 0334ca97c584..7218585681b2 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1057,8 +1057,62 @@ static inline void put_user_page(struct 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); +enum pup_flags_t { + PUP_FLAGS_CLEAN = 0, + PUP_FLAGS_DIRTY = 1, + PUP_FLAGS_LOCK = 2, + PUP_FLAGS_DIRTY_LOCK = 3, +}; + +void __put_user_pages(struct page **pages, unsigned long npages, + enum pup_flags_t flags); + +/** + * put_user_pages_dirty() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * "gup-pinned page" refers to a page that has had one of the get_user_pages() + * variants called on that page. + * + * 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. + * + * set_page_dirty(), which does not lock the page, is used here. + * Therefore, it is the caller's responsibility to ensure that this is + * safe. If not, then put_user_pages_dirty_lock() should be called instead. + * + */ +static inline void put_user_pages_dirty(struct page **pages, + unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY); +} + +/** + * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages + * @pages: array of pages to be marked dirty and released. + * @npages: number of pages in the @pages array. + * + * 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(). + * + */ +static inline void put_user_pages_dirty_lock(struct page **pages, + unsigned long npages) +{ + __put_user_pages(pages, npages, PUP_FLAGS_DIRTY_LOCK); +} + void put_user_pages(struct page **pages, unsigned long npages); #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) diff --git a/mm/gup.c b/mm/gup.c index 98f13ab37bac..6831ef064d76 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -29,87 +29,86 @@ struct follow_page_context { 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() - release an array of gup-pinned pages. * @pages: array of pages to be marked dirty and released. * @npages: number of pages in the @pages array. + * @flags: additional hints, to be applied to each page: * - * "gup-pinned page" refers to a page that has had one of the get_user_pages() - * variants called on that page. + * PUP_FLAGS_CLEAN: no additional steps required. (Consider calling + * put_user_pages() directly, instead.) * - * 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(). + * PUP_FLAGS_DIRTY: Call set_page_dirty() on the page (if not already + * dirty). * - * Please see the put_user_page() documentation for details. + * PUP_FLAGS_LOCK: meaningless by itself, but included in order to show + * the numeric relationship between the flags. * - * set_page_dirty(), which does not lock the page, is used here. - * Therefore, it is the caller's responsibility to ensure that this is - * safe. If not, then put_user_pages_dirty_lock() should be called instead. + * PUP_FLAGS_DIRTY_LOCK: Call set_page_dirty_lock() on the page (if not + * already dirty). * + * For each page in the @pages array, release the page using put_user_page(). */ -void put_user_pages_dirty(struct page **pages, unsigned long npages) +void __put_user_pages(struct page **pages, unsigned long npages, + enum pup_flags_t flags) { - __put_user_pages_dirty(pages, npages, set_page_dirty); -} -EXPORT_SYMBOL(put_user_pages_dirty); + unsigned long index; -/** - * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages - * @pages: array of pages to be marked dirty and released. - * @npages: number of pages in the @pages array. - * - * 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) -{ - __put_user_pages_dirty(pages, npages, set_page_dirty_lock); + /* + * TODO: this can be optimized for huge pages: if a series of pages is + * physically contiguous and part of the same compound page, then a + * single operation to the head page should suffice. + */ + + for (index = 0; index < npages; index++) { + struct page *page = compound_head(pages[index]); + + switch (flags) { + case PUP_FLAGS_CLEAN: + break; + + case PUP_FLAGS_DIRTY: + /* + * 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(page); + break; + + case PUP_FLAGS_LOCK: + VM_WARN_ON_ONCE(flags == PUP_FLAGS_LOCK); + /* + * Shouldn't happen, but treat it as _DIRTY_LOCK if + * it does: fall through. + */ + + case PUP_FLAGS_DIRTY_LOCK: + /* Same comments as for PUP_FLAGS_DIRTY apply here. */ + if (!PageDirty(page)) + set_page_dirty_lock(page); + break; + }; + put_user_page(page); + } } -EXPORT_SYMBOL(put_user_pages_dirty_lock); +EXPORT_SYMBOL(__put_user_pages); /** * put_user_pages() - release an array of gup-pinned pages. -- 2.22.0
next prev parent reply other threads:[~2019-07-22 22:34 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-22 22:34 [PATCH 0/3] introduce __put_user_pages(), convert a few call sites john.hubbard 2019-07-22 22:34 ` john.hubbard 2019-07-22 22:34 ` john.hubbard [this message] 2019-07-22 22:34 ` [PATCH 1/3] mm/gup: introduce __put_user_pages() john.hubbard 2019-07-23 5:53 ` Christoph Hellwig 2019-07-23 6:33 ` John Hubbard 2019-07-23 6:33 ` John Hubbard 2019-07-23 15:36 ` Christoph Hellwig 2019-07-23 15:36 ` Christoph Hellwig 2019-07-22 22:34 ` [PATCH 2/3] drivers/gpu/drm/via: convert put_page() to put_user_page*() john.hubbard 2019-07-22 22:34 ` john.hubbard 2019-07-22 22:34 ` [PATCH 3/3] net/xdp: " john.hubbard 2019-07-22 22:34 ` john.hubbard 2019-07-23 0:25 ` Ira Weiny 2019-07-23 0:25 ` Ira Weiny 2019-07-23 4:41 ` John Hubbard 2019-07-23 4:41 ` John Hubbard 2019-07-23 12:47 ` Jason Gunthorpe 2019-07-23 12:47 ` Jason Gunthorpe 2019-07-23 18:06 ` Ira Weiny 2019-07-23 18:06 ` Ira Weiny 2019-07-23 23:24 ` John Hubbard 2019-07-23 23:24 ` John Hubbard
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190722223415.13269-2-jhubbard@nvidia.com \ --to=john.hubbard@gmail.com \ --cc=airlied@linux.ie \ --cc=akpm@linux-foundation.org \ --cc=axboe@kernel.dk \ --cc=bjorn.topel@intel.com \ --cc=boaz@plexistor.com \ --cc=bpf@vger.kernel.org \ --cc=dan.j.williams@intel.com \ --cc=daniel@ffwll.ch \ --cc=davem@davemloft.net \ --cc=david@fromorbit.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=hch@lst.de \ --cc=idryomov@gmail.com \ --cc=jack@suse.cz \ --cc=jgg@ziepe.ca \ --cc=jglisse@redhat.com \ --cc=jhubbard@nvidia.com \ --cc=jthumshirn@suse.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-rdma@vger.kernel.org \ --cc=magnus.karlsson@intel.com \ --cc=miklos@szeredi.hu \ --cc=ming.lei@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=sage@redhat.com \ --cc=santosh.shilimkar@oracle.com \ --cc=viro@zeniv.linux.org.uk \ --cc=willy@infradead.org \ --cc=zyan@redhat.com \ --subject='Re: [PATCH 1/3] mm/gup: introduce __put_user_pages()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
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.