Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1477639 > unrolled thread

[PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert() Dan Williams <dan.j.williams@intel.com> - 2016-09-06 19:00 +0200
    Re: [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert() Andrew Morton <akpm@linux-foundation.org> - 2016-09-06 22:30 +0200
      Re: [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert() Dan Williams <dan.j.williams@intel.com> - 2016-09-06 22:40 +0200
    Re: [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert() Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-09-07 07:20 +0200
      Re: [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert() Dan Williams <dan.j.williams@intel.com> - 2016-09-07 17:50 +0200

#1477639 — [PATCH 5/5] mm: cleanup pfn_t usage in track_pfn_insert()

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(-)

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);

[toc] | [next] | [standalone]

#1477805

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.

[toc] | [prev] | [next] | [standalone]

#1477812

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"

[toc] | [prev] | [next] | [standalone]

#1477964

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.

[toc] | [prev] | [next] | [standalone]

#1478445

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web