On 03/19/2018 07:00 PM, jglisse@xxxxxxxxxx wrote:
> From: Jérôme Glisse <jglisse@xxxxxxxxxx>
>
> There is no point in differentiating between a range for which there
> is not even a directory (and thus entries) and empty entry (pte_none()
> or pmd_none() returns true).
>
> Simply drop the distinction ie remove HMM_PFN_EMPTY flag and merge now
> duplicate hmm_vma_walk_hole() and hmm_vma_walk_clear() functions.
>
> Changed since v1:
> - Improved comments
>
> Signed-off-by: Jérôme Glisse <jglisse@xxxxxxxxxx>
> Cc: Evgeny Baskakov <ebaskakov@xxxxxxxxxx>
> Cc: Ralph Campbell <rcampbell@xxxxxxxxxx>
> Cc: Mark Hairgrove <mhairgrove@xxxxxxxxxx>
> Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
> include/linux/hmm.h | 8 +++-----
> mm/hmm.c | 45 +++++++++++++++------------------------------
> 2 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 54d684fe3b90..cf283db22106 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -84,7 +84,6 @@ struct hmm;
> * HMM_PFN_VALID: pfn is valid. It has, at least, read permission.
> * HMM_PFN_WRITE: CPU page table has write permission set
> * HMM_PFN_ERROR: corresponding CPU page table entry points to poisoned memory
> - * HMM_PFN_EMPTY: corresponding CPU page table entry is pte_none()
> * HMM_PFN_SPECIAL: corresponding CPU page table entry is special; i.e., the
> * result of vm_insert_pfn() or vm_insert_page(). Therefore, it should not
> * be mirrored by a device, because the entry will never have HMM_PFN_VALID
> @@ -94,10 +93,9 @@ struct hmm;
> #define HMM_PFN_VALID (1 << 0)
> #define HMM_PFN_WRITE (1 << 1)
> #define HMM_PFN_ERROR (1 << 2)
> -#define HMM_PFN_EMPTY (1 << 3)
Hi Jerome,
Nearly done with this one...see below for a bit more detail, but I think if we did this:
#define HMM_PFN_EMPTY (0)
...it would work out nicely.
> -#define HMM_PFN_SPECIAL (1 << 4)
> -#define HMM_PFN_DEVICE_UNADDRESSABLE (1 << 5)
> -#define HMM_PFN_SHIFT 6
> +#define HMM_PFN_SPECIAL (1 << 3)
> +#define HMM_PFN_DEVICE_UNADDRESSABLE (1 << 4)
> +#define HMM_PFN_SHIFT 5
>
<snip>
> @@ -438,7 +423,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
> pfns[i] = 0;
>
> if (pte_none(pte)) {
> - pfns[i] = HMM_PFN_EMPTY;
> + pfns[i] = 0;
This works, but why not keep HMM_PFN_EMPTY, and just define it as zero?
Symbols are better than raw numbers here.
> if (hmm_vma_walk->fault)
> goto fault;
> continue;
> @@ -489,8 +474,8 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>
> fault:
> pte_unmap(ptep);
> - /* Fault all pages in range */
> - return hmm_vma_walk_clear(start, end, walk);
> + /* Fault any virtual address we were ask to fault */
asked to fault
thanks,
--
John Hubbard
NVIDIA
