* Re: [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms.
2020-02-21 21:17 [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms Arjun Roy
@ 2020-02-21 21:25 ` Arjun Roy
2020-02-26 4:22 ` Stephen Rothwell
2020-02-26 23:50 ` Stephen Rothwell
2 siblings, 0 replies; 5+ messages in thread
From: Arjun Roy @ 2020-02-21 21:25 UTC (permalink / raw)
To: Arjun Roy
Cc: David Miller, Andrew Morton, linux-mm, Soheil Hassas Yeganeh,
Eric Dumazet, Stephen Rothwell
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms.
2020-02-21 21:17 [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms Arjun Roy
2020-02-21 21:25 ` Arjun Roy
@ 2020-02-26 4:22 ` Stephen Rothwell
2020-02-26 23:50 ` Stephen Rothwell
2 siblings, 0 replies; 5+ messages in thread
From: Stephen Rothwell @ 2020-02-26 4:22 UTC (permalink / raw)
To: Arjun Roy; +Cc: davem, akpm, linux-mm, arjunroy, soheil, edumazet
[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]
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))
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms.
2020-02-21 21:17 [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms Arjun Roy
2020-02-21 21:25 ` Arjun Roy
2020-02-26 4:22 ` Stephen Rothwell
@ 2020-02-26 23:50 ` Stephen Rothwell
2020-03-01 16:40 ` Mike Rapoport
2 siblings, 1 reply; 5+ messages in thread
From: Stephen Rothwell @ 2020-02-26 23:50 UTC (permalink / raw)
To: Arjun Roy; +Cc: davem, akpm, linux-mm, arjunroy, soheil, edumazet
[-- Attachment #1: Type: text/plain, Size: 4011 bytes --]
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: */
--
2.25.1
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH mm] mm: Bring sparc pte_index() semantics inline with other platforms.
2020-02-26 23:50 ` Stephen Rothwell
@ 2020-03-01 16:40 ` Mike Rapoport
0 siblings, 0 replies; 5+ messages in thread
From: Mike Rapoport @ 2020-03-01 16:40 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Arjun Roy, davem, akpm, linux-mm, arjunroy, soheil, edumazet
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
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 5+ messages in thread