diff mbox series

[mm] mm: Bring sparc pte_index() semantics inline with other platforms.

Message ID 20200221211707.257960-1-arjunroy.kdev@gmail.com (mailing list archive)
State New, archived
Headers show
Series [mm] mm: Bring sparc pte_index() semantics inline with other platforms. | expand

Commit Message

Arjun Roy Feb. 21, 2020, 9:17 p.m. UTC 
From: Arjun Roy <arjunroy@google.com>

pte_index() on platforms other than sparc return a numerical index.
On sparc, it returns a pte_t*. This presents an issue for
vm_insert_pages(), which relies on pte_index() to find the offset for
a pte within a pmd, for batched inserts.

This patch:
1. Modifies pte_index() for sparc to return a numerical index, like
   other platforms,
2. Defines pte_entry() for sparc which returns a pte_t*
   (as pte_index() used to),
3. Converts existing sparc callers for pte_index() to use pte_entry().

Signed-off-by: Arjun Roy <arjunroy@google.com>

---
 arch/sparc/include/asm/pgtable_64.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Arjun Roy Feb. 21, 2020, 9:25 p.m. UTC | #1 
David -

For full context, this is for a patchset I was working on within MM to
reduce spinlock operations for TCP receive zerocopy - which is in and
of itself a bit of a complicated situation since it spans mm and
net-next - but this change specifically was to fix building on sparc
for that patch set.

Stephen (cc'd) suggested that perhaps one route would be to get your
ACK for this patch and have it merged into mm, rather than the sparc
tree.
For that matter, I'm still curious as to the best process for merging
the TCP portion of the patch set - would be happy to hear any ideas
you have. I suggested one possibility may be to delay the TCP portion
till net-next eventually picks up the rest of the changes, and just
change TCP then, but am not sure whether that is the best approach.

Thanks,
-Arjun

On Fri, Feb 21, 2020 at 1:17 PM Arjun Roy <arjunroy.kdev@gmail.com> wrote:
>
> From: Arjun Roy <arjunroy@google.com>
>
> pte_index() on platforms other than sparc return a numerical index.
> On sparc, it returns a pte_t*. This presents an issue for
> vm_insert_pages(), which relies on pte_index() to find the offset for
> a pte within a pmd, for batched inserts.
>
> This patch:
> 1. Modifies pte_index() for sparc to return a numerical index, like
>    other platforms,
> 2. Defines pte_entry() for sparc which returns a pte_t*
>    (as pte_index() used to),
> 3. Converts existing sparc callers for pte_index() to use pte_entry().
>
> Signed-off-by: Arjun Roy <arjunroy@google.com>
>
> ---
>  arch/sparc/include/asm/pgtable_64.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index 65494c3a420e..851ed0f99159 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -907,11 +907,12 @@ static inline unsigned long pud_pfn(pud_t pud)
>          (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
>
>  /* Find an entry in the third-level page table.. */
> -#define pte_index(dir, address)        \
> -       ((pte_t *) __pmd_page(*(dir)) + \
> -        ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
> -#define pte_offset_kernel              pte_index
> -#define pte_offset_map                 pte_index
> +#define pte_index(address)     \
> +        ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> +#define pte_entry(dir, address)        \
> +       ((pte_t *) __pmd_page(*(dir)) + pte_index(address))
> +#define pte_offset_kernel              pte_entry
> +#define pte_offset_map                 pte_entry
>  #define pte_unmap(pte)                 do { } while (0)
>
>  /* We cannot include <linux/mm_types.h> at this point yet: */
> --
> 2.25.0.265.gbab2e86ba0-goog
>
Stephen Rothwell Feb. 26, 2020, 4:22 a.m. UTC | #2 
Hi Arjun,

On Fri, 21 Feb 2020 13:17:07 -0800 Arjun Roy <arjunroy.kdev@gmail.com> wrote:
>
> From: Arjun Roy <arjunroy@google.com>
> 
> pte_index() on platforms other than sparc return a numerical index.
> On sparc, it returns a pte_t*. This presents an issue for
> vm_insert_pages(), which relies on pte_index() to find the offset for
> a pte within a pmd, for batched inserts.
> 
> This patch:
> 1. Modifies pte_index() for sparc to return a numerical index, like
>    other platforms,
> 2. Defines pte_entry() for sparc which returns a pte_t*
>    (as pte_index() used to),

I wonder if you should not just make pte_offset_kernel() do what the
old pte_index() did (this seems to align with what other arches do) and
then make pte_offset_map() be defined to be pte_offset_kernel.

> -#define pte_index(dir, address)	\
> -	((pte_t *) __pmd_page(*(dir)) + \
> -	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
> -#define pte_offset_kernel		pte_index
> -#define pte_offset_map			pte_index
> +#define pte_index(address)	\
> +	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> +#define pte_entry(dir, address)	\
> +	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
> +#define pte_offset_kernel		pte_entry
> +#define pte_offset_map			pte_entry

#define pte_offset_kernel(dir, address)	\
	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
#define pte_offset_map(dir, address)	pte_offset_kernel((dir), (address))
Stephen Rothwell Feb. 26, 2020, 11:50 p.m. UTC | #3 
Hi all,

On Fri, 21 Feb 2020 13:17:07 -0800 Arjun Roy <arjunroy.kdev@gmail.com> wrote:
>
> From: Arjun Roy <arjunroy@google.com>
> 
> pte_index() on platforms other than sparc return a numerical index.
> On sparc, it returns a pte_t*. This presents an issue for
> vm_insert_pages(), which relies on pte_index() to find the offset for
> a pte within a pmd, for batched inserts.
> 
> This patch:
> 1. Modifies pte_index() for sparc to return a numerical index, like
>    other platforms,
> 2. Defines pte_entry() for sparc which returns a pte_t*
>    (as pte_index() used to),
> 3. Converts existing sparc callers for pte_index() to use pte_entry().
> 
> Signed-off-by: Arjun Roy <arjunroy@google.com>
> 
> ---
>  arch/sparc/include/asm/pgtable_64.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index 65494c3a420e..851ed0f99159 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -907,11 +907,12 @@ static inline unsigned long pud_pfn(pud_t pud)
>  	 (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
>  
>  /* Find an entry in the third-level page table.. */
> -#define pte_index(dir, address)	\
> -	((pte_t *) __pmd_page(*(dir)) + \
> -	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
> -#define pte_offset_kernel		pte_index
> -#define pte_offset_map			pte_index
> +#define pte_index(address)	\
> +	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> +#define pte_entry(dir, address)	\
> +	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
> +#define pte_offset_kernel		pte_entry
> +#define pte_offset_map			pte_entry
>  #define pte_unmap(pte)			do { } while (0)
>  
>  /* We cannot include <linux/mm_types.h> at this point yet: */
> -- 
> 2.25.0.265.gbab2e86ba0-goog

I have applied the following patch to the akpm tree today (just ahead
of "mm/memory.c: add vm_insert_pages()") - this includes my suggested
changes.

From: Arjun Roy <arjunroy@google.com>
Date: Fri, 21 Feb 2020 13:17:07 -0800
Subject: [PATCH] mm: Bring sparc pte_index() semantics inline with other platforms.

pte_index() on platforms other than sparc return a numerical index.
On sparc, it returns a pte_t*. This presents an issue for
vm_insert_pages(), which relies on pte_index() to find the offset for
a pte within a pmd, for batched inserts.

This patch:
1. Modifies pte_index() for sparc to return a numerical index, like
   other platforms,
2. Defines pte_entry() for sparc which returns a pte_t*
   (as pte_index() used to),
3. Converts existing sparc callers for pte_index() to use pte_entry().

sfr: removed pte_entry and just directly modified pte_offset_kernel instead

Signed-off-by: Arjun Roy <arjunroy@google.com>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/sparc/include/asm/pgtable_64.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
index 65494c3a420e..930ea3c12739 100644
--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h
@@ -907,11 +907,11 @@ static inline unsigned long pud_pfn(pud_t pud)
 	 (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
 
 /* Find an entry in the third-level page table.. */
-#define pte_index(dir, address)	\
-	((pte_t *) __pmd_page(*(dir)) + \
-	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
-#define pte_offset_kernel		pte_index
-#define pte_offset_map			pte_index
+#define pte_index(address)			\
+	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
+#define pte_offset_kernel(dir, address)	\
+	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
+#define pte_offset_map(dir, address)	pte_offset_kernel((dir), (address))
 #define pte_unmap(pte)			do { } while (0)
 
 /* We cannot include <linux/mm_types.h> at this point yet: */
Mike Rapoport March 1, 2020, 4:40 p.m. UTC | #4 
On Thu, Feb 27, 2020 at 10:50:45AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> On Fri, 21 Feb 2020 13:17:07 -0800 Arjun Roy <arjunroy.kdev@gmail.com> wrote:
> >
> > From: Arjun Roy <arjunroy@google.com>
> > 
> > pte_index() on platforms other than sparc return a numerical index.
> > On sparc, it returns a pte_t*. This presents an issue for
> > vm_insert_pages(), which relies on pte_index() to find the offset for
> > a pte within a pmd, for batched inserts.
> > 
> > This patch:
> > 1. Modifies pte_index() for sparc to return a numerical index, like
> >    other platforms,
> > 2. Defines pte_entry() for sparc which returns a pte_t*
> >    (as pte_index() used to),
> > 3. Converts existing sparc callers for pte_index() to use pte_entry().
> > 
> > Signed-off-by: Arjun Roy <arjunroy@google.com>
> > 
> > ---
> >  arch/sparc/include/asm/pgtable_64.h | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> > index 65494c3a420e..851ed0f99159 100644
> > --- a/arch/sparc/include/asm/pgtable_64.h
> > +++ b/arch/sparc/include/asm/pgtable_64.h
> > @@ -907,11 +907,12 @@ static inline unsigned long pud_pfn(pud_t pud)
> >  	 (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
> >  
> >  /* Find an entry in the third-level page table.. */
> > -#define pte_index(dir, address)	\
> > -	((pte_t *) __pmd_page(*(dir)) + \
> > -	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
> > -#define pte_offset_kernel		pte_index
> > -#define pte_offset_map			pte_index
> > +#define pte_index(address)	\
> > +	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> > +#define pte_entry(dir, address)	\
> > +	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
> > +#define pte_offset_kernel		pte_entry
> > +#define pte_offset_map			pte_entry
> >  #define pte_unmap(pte)			do { } while (0)
> >  
> >  /* We cannot include <linux/mm_types.h> at this point yet: */
> > -- 
> > 2.25.0.265.gbab2e86ba0-goog
> 
> I have applied the following patch to the akpm tree today (just ahead
> of "mm/memory.c: add vm_insert_pages()") - this includes my suggested
> changes.
> 
> From: Arjun Roy <arjunroy@google.com>
> Date: Fri, 21 Feb 2020 13:17:07 -0800
> Subject: [PATCH] mm: Bring sparc pte_index() semantics inline with other platforms.
> 
> pte_index() on platforms other than sparc return a numerical index.
> On sparc, it returns a pte_t*. This presents an issue for
> vm_insert_pages(), which relies on pte_index() to find the offset for
> a pte within a pmd, for batched inserts.
> 
> This patch:
> 1. Modifies pte_index() for sparc to return a numerical index, like
>    other platforms,
> 2. Defines pte_entry() for sparc which returns a pte_t*
>    (as pte_index() used to),
> 3. Converts existing sparc callers for pte_index() to use pte_entry().
> 
> sfr: removed pte_entry and just directly modified pte_offset_kernel instead
> 
> Signed-off-by: Arjun Roy <arjunroy@google.com>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

FWIW: Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  arch/sparc/include/asm/pgtable_64.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index 65494c3a420e..930ea3c12739 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -907,11 +907,11 @@ static inline unsigned long pud_pfn(pud_t pud)
>  	 (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
>  
>  /* Find an entry in the third-level page table.. */
> -#define pte_index(dir, address)	\
> -	((pte_t *) __pmd_page(*(dir)) + \
> -	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
> -#define pte_offset_kernel		pte_index
> -#define pte_offset_map			pte_index
> +#define pte_index(address)			\
> +	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> +#define pte_offset_kernel(dir, address)	\
> +	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
> +#define pte_offset_map(dir, address)	pte_offset_kernel((dir), (address))
>  #define pte_unmap(pte)			do { } while (0)
>  
>  /* We cannot include <linux/mm_types.h> at this point yet: */
> -- 
> 2.25.1
> 
> -- 
> Cheers,
> Stephen Rothwell
diff mbox series

Patch

diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
index 65494c3a420e..851ed0f99159 100644
--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h
@@ -907,11 +907,12 @@  static inline unsigned long pud_pfn(pud_t pud)
 	 (((address) >> PMD_SHIFT) & (PTRS_PER_PMD-1)))
 
 /* Find an entry in the third-level page table.. */
-#define pte_index(dir, address)	\
-	((pte_t *) __pmd_page(*(dir)) + \
-	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1)))
-#define pte_offset_kernel		pte_index
-#define pte_offset_map			pte_index
+#define pte_index(address)	\
+	 ((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
+#define pte_entry(dir, address)	\
+	((pte_t *) __pmd_page(*(dir)) + pte_index(address))
+#define pte_offset_kernel		pte_entry
+#define pte_offset_map			pte_entry
 #define pte_unmap(pte)			do { } while (0)
 
 /* We cannot include <linux/mm_types.h> at this point yet: */