linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Jerome Glisse <jglisse@redhat.com>,
	Ralph Campbell <rcampbell@nvidia.com>,
	Felix.Kuehling@amd.com
Cc: linux-mm@kvack.org, John Hubbard <jhubbard@nvidia.com>,
	dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
	Christoph Hellwig <hch@lst.de>, Philip Yang <Philip.Yang@amd.com>,
	Jason Gunthorpe <jgg@mellanox.com>
Subject: [PATCH  hmm 6/8] mm/hmm: reorganize how !pte_present is handled in hmm_vma_handle_pte()
Date: Wed, 11 Mar 2020 15:35:04 -0300	[thread overview]
Message-ID: <20200311183506.3997-7-jgg@ziepe.ca> (raw)
In-Reply-To: <20200311183506.3997-1-jgg@ziepe.ca>

From: Jason Gunthorpe <jgg@mellanox.com>

The intention with this code is to determine if the caller required the
pages to be valid, and if so, then take some action to make them valid.
The action varies depending on the page type.

In all cases, if the caller doesn't ask for the page, then
hmm_range_fault() should not return an error.

Revise the implementation to be clearer, and fix some bugs:

 - hmm_pte_need_fault() must always be called before testing fault or
   write_fault otherwise the defaults of false apply and the if()'s don't
   work. This was missed on the is_migration_entry() branch

 - -EFAULT should not be returned unless hmm_pte_need_fault() indicates
   fault is required - ie snapshotting should not fail.

 - For !pte_present() the cpu_flags are always 0, except in the special
   case of is_device_private_entry(), calling pte_to_hmm_pfn_flags() is
   confusing.

Reorganize the flow so that it always follows the pattern of calling
hmm_pte_need_fault() and then checking fault || write_fault.

Fixes: 2aee09d8c116 ("mm/hmm: change hmm_vma_fault() to allow write fault on page basis")
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 mm/hmm.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index e10cd0adba7b37..bf676cfef3e8ee 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -282,15 +282,6 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
 	if (!pte_present(pte)) {
 		swp_entry_t entry = pte_to_swp_entry(pte);
 
-		if (!non_swap_entry(entry)) {
-			cpu_flags = pte_to_hmm_pfn_flags(range, pte);
-			hmm_pte_need_fault(hmm_vma_walk, orig_pfn, cpu_flags,
-					   &fault, &write_fault);
-			if (fault || write_fault)
-				goto fault;
-			return 0;
-		}
-
 		/*
 		 * This is a special swap entry, ignore migration, use
 		 * device and report anything else as error.
@@ -310,26 +301,30 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
 			return 0;
 		}
 
-		if (is_migration_entry(entry)) {
-			if (fault || write_fault) {
-				pte_unmap(ptep);
-				hmm_vma_walk->last = addr;
-				migration_entry_wait(walk->mm, pmdp, addr);
-				return -EBUSY;
-			}
+		hmm_pte_need_fault(hmm_vma_walk, orig_pfn, 0, &fault,
+				   &write_fault);
+		if (!fault && !write_fault)
 			return 0;
+
+		if (!non_swap_entry(entry))
+			goto fault;
+
+		if (is_migration_entry(entry)) {
+			pte_unmap(ptep);
+			hmm_vma_walk->last = addr;
+			migration_entry_wait(walk->mm, pmdp, addr);
+			return -EBUSY;
 		}
 
 		/* Report error for everything else */
 		pte_unmap(ptep);
 		*pfn = range->values[HMM_PFN_ERROR];
 		return -EFAULT;
-	} else {
-		cpu_flags = pte_to_hmm_pfn_flags(range, pte);
-		hmm_pte_need_fault(hmm_vma_walk, orig_pfn, cpu_flags,
-				   &fault, &write_fault);
 	}
 
+	cpu_flags = pte_to_hmm_pfn_flags(range, pte);
+	hmm_pte_need_fault(hmm_vma_walk, orig_pfn, cpu_flags, &fault,
+			   &write_fault);
 	if (fault || write_fault)
 		goto fault;
 
-- 
2.25.1



  parent reply	other threads:[~2020-03-11 18:35 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 18:34 [PATCH hmm 0/8] Various error case bug fixes for hmm_range_fault() Jason Gunthorpe
2020-03-11 18:34 ` [PATCH hmm 1/8] mm/hmm: add missing unmaps of the ptep during hmm_vma_handle_pte() Jason Gunthorpe
2020-03-12  1:28   ` Ralph Campbell
2020-03-12 14:24     ` Jason Gunthorpe
2020-03-16  8:58   ` Christoph Hellwig
2020-03-11 18:35 ` [PATCH hmm 2/8] mm/hmm: don't free the cached pgmap while scanning Jason Gunthorpe
2020-03-12  1:29   ` Ralph Campbell
2020-03-16  9:02   ` Christoph Hellwig
2020-03-16 18:07     ` Jason Gunthorpe
2020-03-16 18:13       ` Christoph Hellwig
2020-03-16 19:23         ` Jason Gunthorpe
2020-03-11 18:35 ` [PATCH hmm 3/8] mm/hmm: do not call hmm_vma_walk_hole() while holding a spinlock Jason Gunthorpe
2020-03-12  1:31   ` Ralph Campbell
2020-03-12  8:54   ` Steven Price
2020-03-12 10:28     ` [PATCH] mm/hmm: Simplify hmm_vma_walk_pud slightly Steven Price
2020-03-12 14:27       ` Jason Gunthorpe
2020-03-12 14:40         ` Steven Price
2020-03-12 15:11           ` Jason Gunthorpe
2020-03-12 16:16             ` Steven Price
2020-03-12 16:37               ` Jason Gunthorpe
2020-03-12 17:02                 ` Steven Price
2020-03-12 17:17                   ` Jason Gunthorpe
2020-03-13 19:55                   ` Jason Gunthorpe
2020-03-13 21:04                     ` Matthew Wilcox
2020-03-13 22:51                       ` Jason Gunthorpe
2020-03-16  9:05   ` [PATCH hmm 3/8] mm/hmm: do not call hmm_vma_walk_hole() while holding a spinlock Christoph Hellwig
2020-03-16 12:56     ` Jason Gunthorpe
2020-03-11 18:35 ` [PATCH hmm 4/8] mm/hmm: add missing pfns set to hmm_vma_walk_pmd() Jason Gunthorpe
2020-03-12  1:33   ` Ralph Campbell
2020-03-16  9:06   ` Christoph Hellwig
2020-03-11 18:35 ` [PATCH hmm 5/8] mm/hmm: add missing call to hmm_range_need_fault() before returning EFAULT Jason Gunthorpe
2020-03-12  1:34   ` Ralph Campbell
2020-03-16  9:07   ` Christoph Hellwig
2020-03-11 18:35 ` Jason Gunthorpe [this message]
2020-03-12  1:36   ` [PATCH hmm 6/8] mm/hmm: reorganize how !pte_present is handled in hmm_vma_handle_pte() Ralph Campbell
2020-03-16  9:11   ` Christoph Hellwig
2020-03-11 18:35 ` [PATCH hmm 7/8] mm/hmm: return -EFAULT when setting HMM_PFN_ERROR on requested valid pages Jason Gunthorpe
2020-03-12  1:36   ` Ralph Campbell
2020-03-12 14:35     ` Jason Gunthorpe
2020-03-16  9:12   ` Christoph Hellwig
2020-03-11 18:35 ` [PATCH hmm 8/8] mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling Jason Gunthorpe
2020-03-12  1:38   ` Ralph Campbell
2020-03-16  9:13   ` Christoph Hellwig
2020-03-16 12:10     ` Jason Gunthorpe
2020-03-16 12:49       ` Christoph Hellwig
2020-03-16 13:04         ` Jason Gunthorpe
2020-03-16 13:12           ` Christoph Hellwig
2020-03-17 12:32             ` Christoph Hellwig
2020-03-17 12:53               ` Jason Gunthorpe
2020-03-17 13:06                 ` Christoph Hellwig
2020-03-17 13:25                   ` Jason Gunthorpe
2020-03-16 12:51   ` Christoph Hellwig
2020-03-12 19:33 ` [PATCH hmm 9/8] mm/hmm: do not check pmd_protnone twice in hmm_vma_handle_pmd() Jason Gunthorpe
2020-03-12 23:50   ` Ralph Campbell
2020-03-16  9:14   ` Christoph Hellwig
2020-03-16 18:25 ` [PATCH hmm 0/8] Various error case bug fixes for hmm_range_fault() Jason Gunthorpe

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=20200311183506.3997-7-jgg@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=Felix.Kuehling@amd.com \
    --cc=Philip.Yang@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hch@lst.de \
    --cc=jgg@mellanox.com \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=linux-mm@kvack.org \
    --cc=rcampbell@nvidia.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox