Thread (38 messages) 38 messages, 9 authors, 2018-10-16

Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions

From: Joel Fernandes <hidden>
Date: 2018-10-12 16:34:38
Also in: kvmarm, linux-alpha, linux-mips, linux-mm, linux-riscv, linux-s390, linux-um

On Fri, Oct 12, 2018 at 02:56:19PM +0100, Anton Ivanov wrote:
On 10/12/18 2:37 AM, Joel Fernandes (Google) wrote:
quoted
This series speeds up mremap(2) syscall by copying page tables at the
PMD level even for non-THP systems. There is concern that the extra
'address' argument that mremap passes to pte_alloc may do something
subtle architecture related in the future, that makes the scheme not
work.  Also we find that there is no point in passing the 'address' to
pte_alloc since its unused.

This patch therefore removes this argument tree-wide resulting in a nice
negative diff as well. Also ensuring along the way that the architecture
does not do anything funky with 'address' argument that goes unnoticed.

Build and boot tested on x86-64. Build tested on arm64.

The changes were obtained by applying the following Coccinelle script.
The pte_fragment_alloc was manually fixed up since it was only 2
occurences and could not be easily generalized (and thanks Julia for
answering all my silly and not-silly Coccinelle questions!).

// Options: --include-headers --no-includes
// Note: I split the 'identifier fn' line, so if you are manually
// running it, please unsplit it so it runs for you.

virtual patch

@pte_alloc_func_def depends on patch exists@
identifier E2;
identifier fn =~
"^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
type T2;
@@

  fn(...
- , T2 E2
  )
  { ... }

@pte_alloc_func_proto depends on patch exists@
identifier E1, E2, E4;
type T1, T2, T3, T4;
identifier fn =~
"^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
@@

(
- T3 fn(T1 E1, T2 E2);
+ T3 fn(T1 E1);
|
- T3 fn(T1 E1, T2 E2, T4 E4);
+ T3 fn(T1 E1, T2 E2);
)

@pte_alloc_func_call depends on patch exists@
expression E2;
identifier fn =~
"^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
@@

  fn(...
-,  E2
  )

@pte_alloc_macro depends on patch exists@
identifier fn =~
"^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
identifier a, b, c;
expression e;
position p;
@@

(
- #define fn(a, b, c)@p e
+ #define fn(a, b) e
|
- #define fn(a, b)@p e
+ #define fn(a) e
)

Suggested-by: Kirill A. Shutemov <redacted>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Julia Lawall <redacted>
Cc: elfring@users.sourceforge.net
Signed-off-by: Joel Fernandes (Google) <redacted>
---
  arch/alpha/include/asm/pgalloc.h             |  6 +++---
  arch/arc/include/asm/pgalloc.h               |  5 ++---
  arch/arm/include/asm/pgalloc.h               |  4 ++--
  arch/arm64/include/asm/pgalloc.h             |  4 ++--
  arch/hexagon/include/asm/pgalloc.h           |  6 ++----
  arch/ia64/include/asm/pgalloc.h              |  5 ++---
  arch/m68k/include/asm/mcf_pgalloc.h          |  8 ++------
  arch/m68k/include/asm/motorola_pgalloc.h     |  4 ++--
  arch/m68k/include/asm/sun3_pgalloc.h         |  6 ++----
  arch/microblaze/include/asm/pgalloc.h        | 19 ++-----------------
  arch/microblaze/mm/pgtable.c                 |  3 +--
  arch/mips/include/asm/pgalloc.h              |  6 ++----
  arch/nds32/include/asm/pgalloc.h             |  5 ++---
  arch/nios2/include/asm/pgalloc.h             |  6 ++----
  arch/openrisc/include/asm/pgalloc.h          |  5 ++---
  arch/openrisc/mm/ioremap.c                   |  3 +--
  arch/parisc/include/asm/pgalloc.h            |  4 ++--
  arch/powerpc/include/asm/book3s/32/pgalloc.h |  4 ++--
  arch/powerpc/include/asm/book3s/64/pgalloc.h | 12 +++++-------
  arch/powerpc/include/asm/nohash/32/pgalloc.h |  4 ++--
  arch/powerpc/include/asm/nohash/64/pgalloc.h |  6 ++----
  arch/powerpc/mm/pgtable-book3s64.c           |  2 +-
  arch/powerpc/mm/pgtable_32.c                 |  4 ++--
  arch/riscv/include/asm/pgalloc.h             |  6 ++----
  arch/s390/include/asm/pgalloc.h              |  4 ++--
  arch/sh/include/asm/pgalloc.h                |  6 ++----
  arch/sparc/include/asm/pgalloc_32.h          |  5 ++---
  arch/sparc/include/asm/pgalloc_64.h          |  6 ++----
  arch/sparc/mm/init_64.c                      |  6 ++----
  arch/sparc/mm/srmmu.c                        |  4 ++--
  arch/um/kernel/mem.c                         |  4 ++--
There is a declaration of pte_alloc_one in arch/um/include/asm/pgalloc.h

This patch missed it.
Ah, true. Thanks. Couldn't test every arch obviously. The reason this was
missed is the script could not find matches with prototypes without named
parameters:

extern pgtable_t pte_alloc_one(struct mm_struct *, unsigned long);

I wrote something like this as below but it failed to compile, Julia any
suggestions on how to express this?

@pte_alloc_func_proto depends on patch exists@
type T1, T2, T3, T4;
identifier fn =~
"^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
@@

(
- T3 fn(T1, T2);
+ T3 fn(T1);
|
- T3 fn(T1, T2, T4);
+ T3 fn(T1, T2);
)

thanks,

 - Joel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help