lkml.org 
[lkml]   [2018]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 10/15] mm/hmm: do not differentiate between empty entry or missing directory v2
From
Date
On 03/19/2018 07:00 PM, jglisse@redhat.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
>
> 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@redhat.com>
> Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
> Cc: Ralph Campbell <rcampbell@nvidia.com>
> Cc: Mark Hairgrove <mhairgrove@nvidia.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> ---
> 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

\
 
 \ /
  Last update: 2018-03-21 06:25    [W:0.320 / U:1.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site