linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check
@ 2019-08-21  4:03 John Hubbard
  2019-08-21  4:03 ` [PATCH 2/4] For Ira: tiny formatting tweak to kerneldoc John Hubbard
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: John Hubbard @ 2019-08-21  4:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Ira Weiny,
	Jan Kara, Jason Gunthorpe, Jérôme Glisse,
	Vlastimil Babka, LKML, linux-mm, linux-fsdevel, linux-rdma,
	John Hubbard, Andy Whitcroft, Joe Perches, Gilad Ben-Yossef,
	Ofir Drang

commit 1a47005dd5aa ("checkpatch: add *_NOTIFIER_HEAD as var
definition") causes the following warning when run on some
patches:

Unescaped left brace in regex is passed through in regex;
marked by < --HERE in m/(?:
...
   [238 lines of appalling perl output, mercifully not included]
...
)/ at ./scripts/checkpatch.pl line 3889.

This is broken, so revert it until a better solution is found.

Fixes: 1a47005dd5aa ("checkpatch: add *_NOTIFIER_HEAD as var
definition")

Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Cc: Gilad Ben-Yossef <gilad@benyossef.com>
Cc: Ofir Drang <ofir.drang@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 scripts/checkpatch.pl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5c00151cdee8..284eb4bd84aa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3891,7 +3891,6 @@ sub process {
 				^.DEFINE_$Ident\(\Q$name\E\)|
 				^.DECLARE_$Ident\(\Q$name\E\)|
 				^.LIST_HEAD\(\Q$name\E\)|
-				^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
 				^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
 				\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
 			    )/x) {
-- 
2.22.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/4] For Ira: tiny formatting tweak to kerneldoc
  2019-08-21  4:03 [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
@ 2019-08-21  4:03 ` John Hubbard
  2019-08-21  4:03 ` [PATCH 3/4] mm/gup: introduce FOLL_PIN flag for get_user_pages() John Hubbard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: John Hubbard @ 2019-08-21  4:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Ira Weiny,
	Jan Kara, Jason Gunthorpe, Jérôme Glisse,
	Vlastimil Babka, LKML, linux-mm, linux-fsdevel, linux-rdma,
	John Hubbard

For your vaddr_pin_pages() and vaddr_unpin_pages().
Just merge it into wherever it goes please. Didn't want to
cause merge problems so it's a separate patch-let.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 mm/gup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 56421b880325..e49096d012ea 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2465,7 +2465,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages,
 EXPORT_SYMBOL_GPL(get_user_pages_fast);
 
 /**
- * vaddr_pin_pages pin pages by virtual address and return the pages to the
+ * vaddr_pin_pages() - pin pages by virtual address and return the pages to the
  * user.
  *
  * @addr: start address
@@ -2505,7 +2505,7 @@ long vaddr_pin_pages(unsigned long addr, unsigned long nr_pages,
 EXPORT_SYMBOL(vaddr_pin_pages);
 
 /**
- * vaddr_unpin_pages - counterpart to vaddr_pin_pages
+ * vaddr_unpin_pages() - counterpart to vaddr_pin_pages
  *
  * @pages: array of pages returned
  * @nr_pages: number of pages in pages
-- 
2.22.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/4] mm/gup: introduce FOLL_PIN flag for get_user_pages()
  2019-08-21  4:03 [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
  2019-08-21  4:03 ` [PATCH 2/4] For Ira: tiny formatting tweak to kerneldoc John Hubbard
@ 2019-08-21  4:03 ` John Hubbard
  2019-08-21  4:03 ` [PATCH 4/4] mm/gup: introduce vaddr_pin_pages_remote(), and invoke it John Hubbard
  2019-08-21  4:05 ` disregard: [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
  3 siblings, 0 replies; 5+ messages in thread
From: John Hubbard @ 2019-08-21  4:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Ira Weiny,
	Jan Kara, Jason Gunthorpe, Jérôme Glisse,
	Vlastimil Babka, LKML, linux-mm, linux-fsdevel, linux-rdma,
	John Hubbard, Michal Hocko

FOLL_PIN is set by callers of vaddr_pin_pages(). This is different
than FOLL_LONGTERM, because even short term page pins need a new kind
of tracking, if those pinned pages' data is going to potentially
be modified.

This situation is described in more detail in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

FOLL_PIN is added now, rather than waiting until there is code that
takes action based on FOLL_PIN. That's because having FOLL_PIN in
the code helps to highlight the differences between:

    a) get_user_pages(): soon to be deprecated. Used to pin pages,
       but without awareness of file systems that might use those
       pages,

    b) The original vaddr_pin_pages(): intended only for
       FOLL_LONGTERM and DAX use cases. This assumes direct IO
       and therefore is not applicable the most of the other
       callers of get_user_pages(), and

Also add fairly extensive documentation of the meaning and use
of both FOLL_PIN and FOLL_LONGTERM.

Thanks to Jan Kara and Vlastimil Babka for explaining the 4 cases
in this documentation. (I've reworded it and expanded on it slightly.)

Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jan Kara <jack@suse.cz>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 include/linux/mm.h | 56 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bc675e94ddf8..6e7de424bf5e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2644,6 +2644,8 @@ static inline vm_fault_t vmf_error(int err)
 struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
 			 unsigned int foll_flags);
 
+/* Flags for follow_page(), get_user_pages ("GUP"), and vaddr_pin_pages(): */
+
 #define FOLL_WRITE	0x01	/* check pte is writable */
 #define FOLL_TOUCH	0x02	/* mark page accessed */
 #define FOLL_GET	0x04	/* do get_page on page */
@@ -2663,13 +2665,15 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
 #define FOLL_ANON	0x8000	/* don't do file mappings */
 #define FOLL_LONGTERM	0x10000	/* mapping lifetime is indefinite: see below */
 #define FOLL_SPLIT_PMD	0x20000	/* split huge pmd before returning */
+#define FOLL_PIN	0x40000	/* pages must be released via put_user_page() */
 
 /*
- * NOTE on FOLL_LONGTERM:
+ * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
+ * other. Here is what they mean, and how to use them:
  *
  * FOLL_LONGTERM indicates that the page will be held for an indefinite time
- * period _often_ under userspace control.  This is contrasted with
- * iov_iter_get_pages() where usages which are transient.
+ * period _often_ under userspace control.  This is in contrast to
+ * iov_iter_get_pages(), where usages which are transient.
  *
  * FIXME: For pages which are part of a filesystem, mappings are subject to the
  * lifetime enforced by the filesystem and we need guarantees that longterm
@@ -2684,11 +2688,51 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
  * Currently only get_user_pages() and get_user_pages_fast() support this flag
  * and calls to get_user_pages_[un]locked are specifically not allowed.  This
  * is due to an incompatibility with the FS DAX check and
- * FAULT_FLAG_ALLOW_RETRY
+ * FAULT_FLAG_ALLOW_RETRY.
  *
- * In the CMA case: longterm pins in a CMA region would unnecessarily fragment
- * that region.  And so CMA attempts to migrate the page before pinning when
+ * In the CMA case: long term pins in a CMA region would unnecessarily fragment
+ * that region.  And so, CMA attempts to migrate the page before pinning, when
  * FOLL_LONGTERM is specified.
+ *
+ * FOLL_PIN indicates that a special kind of tracking (not just page->_refcount,
+ * but an additional pin counting system) will be invoked. This is intended for
+ * anything that gets a page reference and then touches page data (for example,
+ * Direct IO). This lets the filesystem know that some non-file-system entity is
+ * potentially changing the pages' data. FOLL_PIN pages must be released,
+ * ultimately, by a call to put_user_page(). Typically that will be via one of
+ * the vaddr_unpin_pages() variants.
+ *
+ * FIXME: note that this special tracking is not in place yet. However, the
+ * pages should still be released by put_user_page().
+ *
+ * When and where to use each flag:
+ *
+ * CASE 1: Direct IO (DIO). There are GUP references to pages that are serving
+ * as DIO buffers. These buffers are needed for a relatively short time (so they
+ * are not "long term"). No special synchronization with page_mkclean() or
+ * munmap() is provided. Therefore, flags to set at the call site are:
+ *
+ *     FOLL_PIN
+ *
+ * CASE 2: RDMA. There are GUP references to pages that are serving as DMA
+ * buffers. These buffers are needed for a long time ("long term"). No special
+ * synchronization with page_mkclean() or munmap() is provided. Therefore, flags
+ * to set at the call site are:
+ *
+ *     FOLL_PIN | FOLL_LONGTERM
+ *
+ * There is also a special case when the pages are DAX pages: in addition to the
+ * above flags, the caller needs a file lease. This is provided via the struct
+ * vaddr_pin argument to vaddr_pin_pages().
+ *
+ * CASE 3: ODP (Mellanox/Infiniband On Demand Paging: the hardware supports
+ * replayable page faulting). There are GUP references to pages serving as DMA
+ * buffers. For ODP, MMU notifiers are used to synchronize with page_mkclean()
+ * and munmap(). Therefore, normal GUP calls are sufficient, so neither flag
+ * needs to be set.
+ *
+ * CASE 4: pinning for struct page manipulation only. Here, normal GUP calls are
+ * sufficient, so neither flag needs to be set.
  */
 
 static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
-- 
2.22.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4/4] mm/gup: introduce vaddr_pin_pages_remote(), and invoke it
  2019-08-21  4:03 [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
  2019-08-21  4:03 ` [PATCH 2/4] For Ira: tiny formatting tweak to kerneldoc John Hubbard
  2019-08-21  4:03 ` [PATCH 3/4] mm/gup: introduce FOLL_PIN flag for get_user_pages() John Hubbard
@ 2019-08-21  4:03 ` John Hubbard
  2019-08-21  4:05 ` disregard: [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
  3 siblings, 0 replies; 5+ messages in thread
From: John Hubbard @ 2019-08-21  4:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Ira Weiny,
	Jan Kara, Jason Gunthorpe, Jérôme Glisse,
	Vlastimil Babka, LKML, linux-mm, linux-fsdevel, linux-rdma,
	John Hubbard

vaddr_pin_user_pages_remote() is the "vaddr_pin_pages" corresponding
variant to get_user_pages_remote(): it adds the ability to handle
FOLL_PIN, FOLL_LONGTERM, or both.

Note that the put_user_page*() requirement won't be truly required until
all of the call sites have been converted, and the tracking of pages is
activated.

Also, change process_vm_rw_single_vec() to invoke the new function.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 include/linux/mm.h     |  5 +++++
 mm/gup.c               | 33 +++++++++++++++++++++++++++++++++
 mm/process_vm_access.c | 23 ++++++++++++++---------
 3 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6e7de424bf5e..849b509e9f89 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1606,6 +1606,11 @@ int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc,
 long vaddr_pin_pages(unsigned long addr, unsigned long nr_pages,
 		     unsigned int gup_flags, struct page **pages,
 		     struct vaddr_pin *vaddr_pin);
+long vaddr_pin_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
+				 unsigned long start, unsigned long nr_pages,
+				 unsigned int gup_flags, struct page **pages,
+				 struct vm_area_struct **vmas, int *locked,
+				 struct vaddr_pin *vaddr_pin);
 void vaddr_unpin_pages(struct page **pages, unsigned long nr_pages,
 		       struct vaddr_pin *vaddr_pin, bool make_dirty);
 bool mapping_inode_has_layout(struct vaddr_pin *vaddr_pin, struct page *page);
diff --git a/mm/gup.c b/mm/gup.c
index e49096d012ea..d7ce9b38178f 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2522,3 +2522,36 @@ void vaddr_unpin_pages(struct page **pages, unsigned long nr_pages,
 	__put_user_pages_dirty_lock(pages, nr_pages, make_dirty, vaddr_pin);
 }
 EXPORT_SYMBOL(vaddr_unpin_pages);
+
+/**
+ * vaddr_pin_user_pages_remote() - pin pages by virtual address and return the
+ * pages to the user.
+ *
+ * @tsk:	the task_struct to use for page fault accounting, or
+ *		NULL if faults are not to be recorded.
+ * @mm:		mm_struct of target mm
+ * @addr:	start address
+ * @nr_pages:	number of pages to pin
+ * @gup_flags:	flags to use for the pin. Please see FOLL_* documentation in
+ *		mm.h.
+ * @pages:	array of pages returned
+ * @vaddr_pin:  If FOLL_LONGTERM is set, then vaddr_pin should point to an
+ * initialized struct that contains the owning mm and file. Otherwise, vaddr_pin
+ * should be set to NULL.
+ *
+ * This is the "vaddr_pin_pages" corresponding variant to
+ * get_user_pages_remote(), but with the ability to handle FOLL_PIN,
+ * FOLL_LONGTERM, or both.
+ */
+long vaddr_pin_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
+				 unsigned long start, unsigned long nr_pages,
+				 unsigned int gup_flags, struct page **pages,
+				 struct vm_area_struct **vmas, int *locked,
+				 struct vaddr_pin *vaddr_pin)
+{
+	gup_flags |= FOLL_TOUCH | FOLL_REMOTE;
+
+	return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas,
+				       locked, gup_flags, vaddr_pin);
+}
+EXPORT_SYMBOL(vaddr_pin_user_pages_remote);
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 357aa7bef6c0..e08c1f760ad4 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -96,7 +96,7 @@ static int process_vm_rw_single_vec(unsigned long addr,
 		flags |= FOLL_WRITE;
 
 	while (!rc && nr_pages && iov_iter_count(iter)) {
-		int pages = min(nr_pages, max_pages_per_loop);
+		int pinned_pages = min(nr_pages, max_pages_per_loop);
 		int locked = 1;
 		size_t bytes;
 
@@ -106,14 +106,18 @@ static int process_vm_rw_single_vec(unsigned long addr,
 		 * current/current->mm
 		 */
 		down_read(&mm->mmap_sem);
-		pages = get_user_pages_remote(task, mm, pa, pages, flags,
-					      process_pages, NULL, &locked);
+
+		flags |= FOLL_PIN;
+		pinned_pages = vaddr_pin_user_pages_remote(task, mm, pa,
+							   pinned_pages, flags,
+							   process_pages, NULL,
+							   &locked, NULL);
 		if (locked)
 			up_read(&mm->mmap_sem);
-		if (pages <= 0)
+		if (pinned_pages <= 0)
 			return -EFAULT;
 
-		bytes = pages * PAGE_SIZE - start_offset;
+		bytes = pinned_pages * PAGE_SIZE - start_offset;
 		if (bytes > len)
 			bytes = len;
 
@@ -122,10 +126,11 @@ static int process_vm_rw_single_vec(unsigned long addr,
 					 vm_write);
 		len -= bytes;
 		start_offset = 0;
-		nr_pages -= pages;
-		pa += pages * PAGE_SIZE;
-		while (pages)
-			put_page(process_pages[--pages]);
+		nr_pages -= pinned_pages;
+		pa += pinned_pages * PAGE_SIZE;
+
+		/* If vm_write is set, the pages need to be made dirty: */
+		vaddr_unpin_pages(process_pages, pinned_pages, NULL, vm_write);
 	}
 
 	return rc;
-- 
2.22.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* disregard: [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check
  2019-08-21  4:03 [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
                   ` (2 preceding siblings ...)
  2019-08-21  4:03 ` [PATCH 4/4] mm/gup: introduce vaddr_pin_pages_remote(), and invoke it John Hubbard
@ 2019-08-21  4:05 ` John Hubbard
  3 siblings, 0 replies; 5+ messages in thread
From: John Hubbard @ 2019-08-21  4:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Ira Weiny,
	Jan Kara, Jason Gunthorpe, Jérôme Glisse,
	Vlastimil Babka, LKML, linux-mm, linux-fsdevel, linux-rdma,
	Andy Whitcroft, Joe Perches, Gilad Ben-Yossef, Ofir Drang

On 8/20/19 9:03 PM, John Hubbard wrote:
> commit 1a47005dd5aa ("checkpatch: add *_NOTIFIER_HEAD as var
> definition") causes the following warning when run on some
> patches:
> 

Please disregard this series. It's stale.

thanks,
-- 
John Hubbard
NVIDIA

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-08-21  4:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21  4:03 [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard
2019-08-21  4:03 ` [PATCH 2/4] For Ira: tiny formatting tweak to kerneldoc John Hubbard
2019-08-21  4:03 ` [PATCH 3/4] mm/gup: introduce FOLL_PIN flag for get_user_pages() John Hubbard
2019-08-21  4:03 ` [PATCH 4/4] mm/gup: introduce vaddr_pin_pages_remote(), and invoke it John Hubbard
2019-08-21  4:05 ` disregard: [PATCH 1/4] checkpatch: revert broken NOTIFIER_HEAD check John Hubbard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).