| Message ID | 147318058712.30325.12749411762275637099.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 06 Sep 2016 09:49:47 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > Now that track_pfn_insert() is no longer used in the DAX path, it no > longer needs to comprehend pfn_t values. What's the benefit in this? A pfn *should* have type pfn_t, shouldn't it? Confused.
On Tue, Sep 6, 2016 at 1:20 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 06 Sep 2016 09:49:47 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > >> Now that track_pfn_insert() is no longer used in the DAX path, it no >> longer needs to comprehend pfn_t values. > > What's the benefit in this? A pfn *should* have type pfn_t, shouldn't > it? Confused. It should when there's extra information to consider. I don't mind leaving it as is, but all the other usages of pfn_t are considering or passing through the PFN_DEV and PFN_MAP flags. So, it's a courtesy to the reader saying "you don't need to worry about pfn_t defined behavior here, this is just a plain old physical address >> PAGE_SHIFT"
On 09/06/2016 10:19 PM, Dan Williams wrote: > Now that track_pfn_insert() is no longer used in the DAX path, it no > longer needs to comprehend pfn_t values. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > arch/x86/mm/pat.c | 4 ++-- > include/asm-generic/pgtable.h | 4 ++-- > mm/memory.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) A small nit. Should not the arch/x86/mm/pat.c changes be separated out into a different patch ? Kind of faced little bit problem separating out generic core mm changes to that of arch specific mm changes when going through the commits in retrospect.
On Tue, Sep 6, 2016 at 10:12 PM, Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote: > On 09/06/2016 10:19 PM, Dan Williams wrote: >> Now that track_pfn_insert() is no longer used in the DAX path, it no >> longer needs to comprehend pfn_t values. >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> arch/x86/mm/pat.c | 4 ++-- >> include/asm-generic/pgtable.h | 4 ++-- >> mm/memory.c | 2 +- >> 3 files changed, 5 insertions(+), 5 deletions(-) > > A small nit. Should not the arch/x86/mm/pat.c changes be separated out > into a different patch ? Kind of faced little bit problem separating out > generic core mm changes to that of arch specific mm changes when going > through the commits in retrospect. I'm going to drop this change. Leaving it as is does no harm, and users of pfn_t are likely to grow over time.
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c index ecb1b69c1651..e8aed3a30e29 100644 --- a/arch/x86/mm/pat.c +++ b/arch/x86/mm/pat.c @@ -971,7 +971,7 @@ int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot, } int track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot, - pfn_t pfn) + unsigned long pfn) { enum page_cache_mode pcm; @@ -979,7 +979,7 @@ int track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot, return 0; /* Set prot based on lookup */ - pcm = lookup_memtype(pfn_t_to_phys(pfn)); + pcm = lookup_memtype(PFN_PHYS(pfn)); *prot = __pgprot((pgprot_val(*prot) & (~_PAGE_CACHE_MASK)) | cachemode2protval(pcm)); diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h index d4458b6dbfb4..f9a4f708227d 100644 --- a/include/asm-generic/pgtable.h +++ b/include/asm-generic/pgtable.h @@ -559,7 +559,7 @@ static inline int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot, * by vm_insert_pfn(). */ static inline int track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot, - pfn_t pfn) + unsigned long pfn) { return 0; } @@ -594,7 +594,7 @@ extern int track_pfn_remap(struct vm_area_struct *vma, pgprot_t *prot, unsigned long pfn, unsigned long addr, unsigned long size); extern int track_pfn_insert(struct vm_area_struct *vma, pgprot_t *prot, - pfn_t pfn); + unsigned long pfn); extern int track_pfn_copy(struct vm_area_struct *vma); extern void untrack_pfn(struct vm_area_struct *vma, unsigned long pfn, unsigned long size); diff --git a/mm/memory.c b/mm/memory.c index 83be99d9d8a1..5d4826a28e3f 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1637,7 +1637,7 @@ int vm_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr, if (addr < vma->vm_start || addr >= vma->vm_end) return -EFAULT; - if (track_pfn_insert(vma, &pgprot, __pfn_to_pfn_t(pfn, PFN_DEV))) + if (track_pfn_insert(vma, &pgprot, pfn)) return -EINVAL; ret = insert_pfn(vma, addr, __pfn_to_pfn_t(pfn, PFN_DEV), pgprot);
Now that track_pfn_insert() is no longer used in the DAX path, it no longer needs to comprehend pfn_t values. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- arch/x86/mm/pat.c | 4 ++-- include/asm-generic/pgtable.h | 4 ++-- mm/memory.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)