Recently the get_unmapped_area() pointer on mm_struct was removed in
favor of direct callable function that can determines which of two
handlers to call based on an mm flag. This function,
mm_get_unmapped_area(), checks the flag of the mm passed as an argument.
Dan Williams pointed out (see link) that all callers pass curret->mm, so
the mm argument is unneeded. It could be conceivable for a caller to want
to pass a different mm in the future, but in this case a new helper could
easily be added.
So remove the mm argument, and rename the function
current_get_unmapped_area().
Fixes: 529ce23a764f ("mm: switch mm->get_unmapped_area() to a flag")
Suggested-by: Dan Williams <[email protected]>
Signed-off-by: Rick Edgecombe <[email protected]>
Link: https://lore.kernel.org/lkml/6603bed6662a_4a98a2949e@dwillia2-mobl3.amr.corp.intel.com.notmuch/
---
Based on linux-next.
---
arch/sparc/kernel/sys_sparc_64.c | 9 +++++----
arch/x86/kernel/cpu/sgx/driver.c | 2 +-
drivers/char/mem.c | 2 +-
drivers/dax/device.c | 6 +++---
fs/proc/inode.c | 2 +-
fs/ramfs/file-mmu.c | 2 +-
include/linux/sched/mm.h | 6 +++---
io_uring/memmap.c | 2 +-
kernel/bpf/arena.c | 2 +-
kernel/bpf/syscall.c | 2 +-
mm/mmap.c | 11 +++++------
mm/shmem.c | 9 ++++-----
12 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index d9c3b34ca744..cf0b4ace5bf9 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -220,7 +220,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
if (flags & MAP_FIXED) {
/* Ok, don't mess with it. */
- return mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
+ return current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
}
flags &= ~MAP_SHARED;
@@ -233,8 +233,9 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
align_goal = (64UL * 1024);
do {
- addr = mm_get_unmapped_area(current->mm, NULL, orig_addr,
- len + (align_goal - PAGE_SIZE), pgoff, flags);
+ addr = current_get_unmapped_area(NULL, orig_addr,
+ len + (align_goal - PAGE_SIZE),
+ pgoff, flags);
if (!(addr & ~PAGE_MASK)) {
addr = (addr + (align_goal - 1UL)) & ~(align_goal - 1UL);
break;
@@ -252,7 +253,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
* be obtained.
*/
if (addr & ~PAGE_MASK)
- addr = mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
+ addr = current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
return addr;
}
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index 22b65a5f5ec6..5f7bfd9035f7 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -113,7 +113,7 @@ static unsigned long sgx_get_unmapped_area(struct file *file,
if (flags & MAP_FIXED)
return addr;
- return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
+ return current_get_unmapped_area(file, addr, len, pgoff, flags);
}
#ifdef CONFIG_COMPAT
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 7c359cc406d5..a29c4bd506d5 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -546,7 +546,7 @@ static unsigned long get_unmapped_area_zero(struct file *file,
}
/* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
- return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
+ return current_get_unmapped_area(file, addr, len, pgoff, flags);
#else
return -ENOSYS;
#endif
diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index eb61598247a9..c379902307b7 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -329,14 +329,14 @@ static unsigned long dax_get_unmapped_area(struct file *filp,
if ((off + len_align) < off)
goto out;
- addr_align = mm_get_unmapped_area(current->mm, filp, addr, len_align,
- pgoff, flags);
+ addr_align = current_get_unmapped_area(filp, addr, len_align,
+ pgoff, flags);
if (!IS_ERR_VALUE(addr_align)) {
addr_align += (off - addr_align) & (align - 1);
return addr_align;
}
out:
- return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
+ return current_get_unmapped_area(filp, addr, len, pgoff, flags);
}
static const struct address_space_operations dev_dax_aops = {
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index d19434e2a58e..24a6aeac3de5 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -455,7 +455,7 @@ pde_get_unmapped_area(struct proc_dir_entry *pde, struct file *file, unsigned lo
return pde->proc_ops->proc_get_unmapped_area(file, orig_addr, len, pgoff, flags);
#ifdef CONFIG_MMU
- return mm_get_unmapped_area(current->mm, file, orig_addr, len, pgoff, flags);
+ return current_get_unmapped_area(file, orig_addr, len, pgoff, flags);
#endif
return orig_addr;
diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
index b45c7edc3225..85f57de31102 100644
--- a/fs/ramfs/file-mmu.c
+++ b/fs/ramfs/file-mmu.c
@@ -35,7 +35,7 @@ static unsigned long ramfs_mmu_get_unmapped_area(struct file *file,
unsigned long addr, unsigned long len, unsigned long pgoff,
unsigned long flags)
{
- return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
+ return current_get_unmapped_area(file, addr, len, pgoff, flags);
}
const struct file_operations ramfs_file_operations = {
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 91546493c43d..c67c7de05c7a 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -187,9 +187,9 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
unsigned long flags);
-unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
- unsigned long addr, unsigned long len,
- unsigned long pgoff, unsigned long flags);
+unsigned long current_get_unmapped_area(struct file *filp, unsigned long addr,
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags);
unsigned long
arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
diff --git a/io_uring/memmap.c b/io_uring/memmap.c
index 4785d6af5fee..1aaea32c797c 100644
--- a/io_uring/memmap.c
+++ b/io_uring/memmap.c
@@ -305,7 +305,7 @@ unsigned long io_uring_get_unmapped_area(struct file *filp, unsigned long addr,
#else
addr = 0UL;
#endif
- return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
+ return current_get_unmapped_area(filp, addr, len, pgoff, flags);
}
#else /* !CONFIG_MMU */
diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 4a1be699bb82..054486f7c453 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -314,7 +314,7 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
return -EINVAL;
}
- ret = mm_get_unmapped_area(current->mm, filp, addr, len * 2, 0, flags);
+ ret = current_get_unmapped_area(filp, addr, len * 2, 0, flags);
if (IS_ERR_VALUE(ret))
return ret;
if ((ret >> 32) == ((ret + len - 1) >> 32))
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2222c3ff88e7..d9ff2843f6ef 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -992,7 +992,7 @@ static unsigned long bpf_get_unmapped_area(struct file *filp, unsigned long addr
if (map->ops->map_get_unmapped_area)
return map->ops->map_get_unmapped_area(filp, addr, len, pgoff, flags);
#ifdef CONFIG_MMU
- return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
+ return current_get_unmapped_area(filp, addr, len, pgoff, flags);
#else
return addr;
#endif
diff --git a/mm/mmap.c b/mm/mmap.c
index 83b4682ec85c..4e98a907c53d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1901,16 +1901,15 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
return error ? error : addr;
}
-unsigned long
-mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
- unsigned long addr, unsigned long len,
- unsigned long pgoff, unsigned long flags)
+unsigned long current_get_unmapped_area(struct file *file, unsigned long addr,
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags)
{
- if (test_bit(MMF_TOPDOWN, &mm->flags))
+ if (test_bit(MMF_TOPDOWN, ¤t->mm->flags))
return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
return arch_get_unmapped_area(file, addr, len, pgoff, flags);
}
-EXPORT_SYMBOL(mm_get_unmapped_area);
+EXPORT_SYMBOL(current_get_unmapped_area);
/**
* find_vma_intersection() - Look up the first VMA which intersects the interval
diff --git a/mm/shmem.c b/mm/shmem.c
index f5d60436b604..c0acd7db93c8 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2276,8 +2276,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
if (len > TASK_SIZE)
return -ENOMEM;
- addr = mm_get_unmapped_area(current->mm, file, uaddr, len, pgoff,
- flags);
+ addr = current_get_unmapped_area(file, uaddr, len, pgoff, flags);
if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
return addr;
@@ -2334,8 +2333,8 @@ unsigned long shmem_get_unmapped_area(struct file *file,
if (inflated_len < len)
return addr;
- inflated_addr = mm_get_unmapped_area(current->mm, NULL, uaddr,
- inflated_len, 0, flags);
+ inflated_addr = current_get_unmapped_area(NULL, uaddr,
+ inflated_len, 0, flags);
if (IS_ERR_VALUE(inflated_addr))
return addr;
if (inflated_addr & ~PAGE_MASK)
@@ -4799,7 +4798,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags)
{
- return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
+ return current_get_unmapped_area(file, addr, len, pgoff, flags);
}
#endif
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
--
2.34.1
* Rick Edgecombe <[email protected]> [240506 12:08]:
> Recently the get_unmapped_area() pointer on mm_struct was removed in
> favor of direct callable function that can determines which of two
> handlers to call based on an mm flag. This function,
> mm_get_unmapped_area(), checks the flag of the mm passed as an argument.
>
> Dan Williams pointed out (see link) that all callers pass curret->mm, so
> the mm argument is unneeded. It could be conceivable for a caller to want
> to pass a different mm in the future, but in this case a new helper could
> easily be added.
>
> So remove the mm argument, and rename the function
> current_get_unmapped_area().
I like this patch.
I think the context of current->mm is implied. IOW, could we call it
get_unmapped_area() instead? There are other functions today that use
current->mm that don't start with current_<whatever>. I probably should
have responded to Dan's suggestion with my comment.
Either way, this is a minor thing so feel free to add:
Reviewed-by: Liam R. Howlett <[email protected]>
>
> Fixes: 529ce23a764f ("mm: switch mm->get_unmapped_area() to a flag")
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Rick Edgecombe <[email protected]>
> Link: https://lore.kernel.org/lkml/6603bed6662a_4a98a2949e@dwillia2-mobl3.amr.corp.intel.com.notmuch/
> ---
> Based on linux-next.
> ---
> arch/sparc/kernel/sys_sparc_64.c | 9 +++++----
> arch/x86/kernel/cpu/sgx/driver.c | 2 +-
> drivers/char/mem.c | 2 +-
> drivers/dax/device.c | 6 +++---
> fs/proc/inode.c | 2 +-
> fs/ramfs/file-mmu.c | 2 +-
> include/linux/sched/mm.h | 6 +++---
> io_uring/memmap.c | 2 +-
> kernel/bpf/arena.c | 2 +-
> kernel/bpf/syscall.c | 2 +-
> mm/mmap.c | 11 +++++------
> mm/shmem.c | 9 ++++-----
> 12 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index d9c3b34ca744..cf0b4ace5bf9 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -220,7 +220,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
>
> if (flags & MAP_FIXED) {
> /* Ok, don't mess with it. */
> - return mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
> + return current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
> }
> flags &= ~MAP_SHARED;
>
> @@ -233,8 +233,9 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
> align_goal = (64UL * 1024);
>
> do {
> - addr = mm_get_unmapped_area(current->mm, NULL, orig_addr,
> - len + (align_goal - PAGE_SIZE), pgoff, flags);
> + addr = current_get_unmapped_area(NULL, orig_addr,
> + len + (align_goal - PAGE_SIZE),
> + pgoff, flags);
> if (!(addr & ~PAGE_MASK)) {
> addr = (addr + (align_goal - 1UL)) & ~(align_goal - 1UL);
> break;
> @@ -252,7 +253,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
> * be obtained.
> */
> if (addr & ~PAGE_MASK)
> - addr = mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
> + addr = current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
>
> return addr;
> }
> diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
> index 22b65a5f5ec6..5f7bfd9035f7 100644
> --- a/arch/x86/kernel/cpu/sgx/driver.c
> +++ b/arch/x86/kernel/cpu/sgx/driver.c
> @@ -113,7 +113,7 @@ static unsigned long sgx_get_unmapped_area(struct file *file,
> if (flags & MAP_FIXED)
> return addr;
>
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
>
> #ifdef CONFIG_COMPAT
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 7c359cc406d5..a29c4bd506d5 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -546,7 +546,7 @@ static unsigned long get_unmapped_area_zero(struct file *file,
> }
>
> /* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> #else
> return -ENOSYS;
> #endif
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index eb61598247a9..c379902307b7 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -329,14 +329,14 @@ static unsigned long dax_get_unmapped_area(struct file *filp,
> if ((off + len_align) < off)
> goto out;
>
> - addr_align = mm_get_unmapped_area(current->mm, filp, addr, len_align,
> - pgoff, flags);
> + addr_align = current_get_unmapped_area(filp, addr, len_align,
> + pgoff, flags);
> if (!IS_ERR_VALUE(addr_align)) {
> addr_align += (off - addr_align) & (align - 1);
> return addr_align;
> }
> out:
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
>
> static const struct address_space_operations dev_dax_aops = {
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index d19434e2a58e..24a6aeac3de5 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -455,7 +455,7 @@ pde_get_unmapped_area(struct proc_dir_entry *pde, struct file *file, unsigned lo
> return pde->proc_ops->proc_get_unmapped_area(file, orig_addr, len, pgoff, flags);
>
> #ifdef CONFIG_MMU
> - return mm_get_unmapped_area(current->mm, file, orig_addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, orig_addr, len, pgoff, flags);
> #endif
>
> return orig_addr;
> diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
> index b45c7edc3225..85f57de31102 100644
> --- a/fs/ramfs/file-mmu.c
> +++ b/fs/ramfs/file-mmu.c
> @@ -35,7 +35,7 @@ static unsigned long ramfs_mmu_get_unmapped_area(struct file *file,
> unsigned long addr, unsigned long len, unsigned long pgoff,
> unsigned long flags)
> {
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
>
> const struct file_operations ramfs_file_operations = {
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 91546493c43d..c67c7de05c7a 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -187,9 +187,9 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags);
>
> -unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
> - unsigned long addr, unsigned long len,
> - unsigned long pgoff, unsigned long flags);
> +unsigned long current_get_unmapped_area(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags);
>
> unsigned long
> arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
> diff --git a/io_uring/memmap.c b/io_uring/memmap.c
> index 4785d6af5fee..1aaea32c797c 100644
> --- a/io_uring/memmap.c
> +++ b/io_uring/memmap.c
> @@ -305,7 +305,7 @@ unsigned long io_uring_get_unmapped_area(struct file *filp, unsigned long addr,
> #else
> addr = 0UL;
> #endif
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
>
> #else /* !CONFIG_MMU */
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 4a1be699bb82..054486f7c453 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -314,7 +314,7 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
> return -EINVAL;
> }
>
> - ret = mm_get_unmapped_area(current->mm, filp, addr, len * 2, 0, flags);
> + ret = current_get_unmapped_area(filp, addr, len * 2, 0, flags);
> if (IS_ERR_VALUE(ret))
> return ret;
> if ((ret >> 32) == ((ret + len - 1) >> 32))
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2222c3ff88e7..d9ff2843f6ef 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -992,7 +992,7 @@ static unsigned long bpf_get_unmapped_area(struct file *filp, unsigned long addr
> if (map->ops->map_get_unmapped_area)
> return map->ops->map_get_unmapped_area(filp, addr, len, pgoff, flags);
> #ifdef CONFIG_MMU
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> #else
> return addr;
> #endif
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 83b4682ec85c..4e98a907c53d 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1901,16 +1901,15 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
> return error ? error : addr;
> }
>
> -unsigned long
> -mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
> - unsigned long addr, unsigned long len,
> - unsigned long pgoff, unsigned long flags)
> +unsigned long current_get_unmapped_area(struct file *file, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags)
> {
> - if (test_bit(MMF_TOPDOWN, &mm->flags))
> + if (test_bit(MMF_TOPDOWN, ¤t->mm->flags))
> return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
> return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> }
> -EXPORT_SYMBOL(mm_get_unmapped_area);
> +EXPORT_SYMBOL(current_get_unmapped_area);
>
> /**
> * find_vma_intersection() - Look up the first VMA which intersects the interval
> diff --git a/mm/shmem.c b/mm/shmem.c
> index f5d60436b604..c0acd7db93c8 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2276,8 +2276,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> if (len > TASK_SIZE)
> return -ENOMEM;
>
> - addr = mm_get_unmapped_area(current->mm, file, uaddr, len, pgoff,
> - flags);
> + addr = current_get_unmapped_area(file, uaddr, len, pgoff, flags);
>
> if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> return addr;
> @@ -2334,8 +2333,8 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> if (inflated_len < len)
> return addr;
>
> - inflated_addr = mm_get_unmapped_area(current->mm, NULL, uaddr,
> - inflated_len, 0, flags);
> + inflated_addr = current_get_unmapped_area(NULL, uaddr,
> + inflated_len, 0, flags);
> if (IS_ERR_VALUE(inflated_addr))
> return addr;
> if (inflated_addr & ~PAGE_MASK)
> @@ -4799,7 +4798,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags)
> {
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
> #endif
>
>
> base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
> --
> 2.34.1
>
On Mon, May 06, 2024 at 09:07:47AM -0700, Rick Edgecombe wrote:
> if (flags & MAP_FIXED) {
> /* Ok, don't mess with it. */
> - return mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
> + return current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
The old name seems preferable because it's not as crazy long. In fact
just get_unmapped_area would be even better, but that's already taken
by something else.
Can we maybe take a step back and sort out the mess of the various
_get_unmapped_area helpers?
e.g. mm_get_unmapped_area_vmflags just wraps
arch_get_unmapped_area_topdown_vmflags and
arch_get_unmapped_area_vmflags, and we might as well merge all three
by moving the MMF_TOPDOWN into two actual implementations?
And then just update all the implementations to always pass the
vm_flags instead of having separate implementations with our without
the flags.
And then make __get_unmapped_area static in mmap.c nad move the
get_unmapped_area wrappers there. And eventually write some
documentation for the functions based on the learnings who actually
uses what..
On Mon, 2024-05-06 at 12:32 -0400, Liam R. Howlett wrote:
>
> I like this patch.
Thanks for taking a look.
>
> I think the context of current->mm is implied. IOW, could we call it
> get_unmapped_area() instead? There are other functions today that use
> current->mm that don't start with current_<whatever>. I probably should
> have responded to Dan's suggestion with my comment.
Yes, get_unmapped_area() is already taken. What else to call it... It is kind of
the process "default" get_unmapped_area(). But with Christoph's proposal it
would basically be arch_get_unmapped_area().
>
> Either way, this is a minor thing so feel free to add:
> Reviewed-by: Liam R. Howlett <[email protected]>
On Mon, 2024-05-06 at 09:18 -0700, Christoph Hellwig wrote:
> > On Mon, May 06, 2024 at 09:07:47AM -0700, Rick Edgecombe wrote:
> > > > if (flags & MAP_FIXED) {
> > > > /* Ok, don't mess with it. */
> > > > - return mm_get_unmapped_area(current->mm, NULL,
> > > > orig_addr, > > len, pgoff, flags);
> > > > + return current_get_unmapped_area(NULL, orig_addr, len, >
> > > > > pgoff, flags);
> >
> > The old name seems preferable because it's not as crazy long. In fact
> > just get_unmapped_area would be even better, but that's already taken
> > by something else.
Ok.
> >
> > Can we maybe take a step back and sort out the mess of the various
> > _get_unmapped_area helpers?
> >
> > e.g. mm_get_unmapped_area_vmflags just wraps
> > arch_get_unmapped_area_topdown_vmflags and
> > arch_get_unmapped_area_vmflags, and we might as well merge all three
> > by moving the MMF_TOPDOWN into two actual implementations?
> >
> > And then just update all the implementations to always pass the
> > vm_flags instead of having separate implementations with our without
> > the flags.
> >
> > And then make __get_unmapped_area static in mmap.c nad move the
> > get_unmapped_area wrappers there. And eventually write some
> > documentation for the functions based on the learnings who actually
> > uses what..
The rest of the series[0] is in the mm-tree/linux-next currently. Are you
suggesting we not do this patch, and leave the rest you describe here for the
future? I think the removal of the indirect branch is at least a positive step
forward.
[0]
https://lore.kernel.org/lkml/[email protected]/
* Edgecombe, Rick P <[email protected]> [240507 09:51]:
> On Mon, 2024-05-06 at 12:32 -0400, Liam R. Howlett wrote:
> >
> > I like this patch.
>
> Thanks for taking a look.
>
> >
> > I think the context of current->mm is implied. IOW, could we call it
> > get_unmapped_area() instead?? There are other functions today that use
> > current->mm that don't start with current_<whatever>.? I probably should
> > have responded to Dan's suggestion with my comment.
>
> Yes, get_unmapped_area() is already taken. What else to call it... It is kind of
> the process "default" get_unmapped_area(). But with Christoph's proposal it
> would basically be arch_get_unmapped_area().
unmapped_area(), but that's also taken..
arch_get_unmapped_area() are all quite close. If you look into it, many
of the arch versions were taken from the sparc 32 version. Subsequent
changes were made and they are no longer exactly the same, but I believe
functionally equivalent - rather tricky to test though.
I wanted to unite these to simplify the mm code a while back, but have
not gotten back to it. One aspect that some archs have is "cache
coloring" which does affect the VMAs.
The other difference is VDSO, which I may be looking into soon. Someone
once called me a glutton for punishment and there may be some truth in
that...
Cheers,
Liam
On Mon May 6, 2024 at 7:07 PM EEST, Rick Edgecombe wrote:
> Recently the get_unmapped_area() pointer on mm_struct was removed in
> favor of direct callable function that can determines which of two
> handlers to call based on an mm flag. This function,
> mm_get_unmapped_area(), checks the flag of the mm passed as an argument.
>
> Dan Williams pointed out (see link) that all callers pass curret->mm, so
> the mm argument is unneeded. It could be conceivable for a caller to want
> to pass a different mm in the future, but in this case a new helper could
> easily be added.
>
> So remove the mm argument, and rename the function
> current_get_unmapped_area().
>
> Fixes: 529ce23a764f ("mm: switch mm->get_unmapped_area() to a flag")
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Rick Edgecombe <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> ---
> Based on linux-next.
> ---
> arch/sparc/kernel/sys_sparc_64.c | 9 +++++----
> arch/x86/kernel/cpu/sgx/driver.c | 2 +-
> drivers/char/mem.c | 2 +-
> drivers/dax/device.c | 6 +++---
> fs/proc/inode.c | 2 +-
> fs/ramfs/file-mmu.c | 2 +-
> include/linux/sched/mm.h | 6 +++---
> io_uring/memmap.c | 2 +-
> kernel/bpf/arena.c | 2 +-
> kernel/bpf/syscall.c | 2 +-
> mm/mmap.c | 11 +++++------
> mm/shmem.c | 9 ++++-----
> 12 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index d9c3b34ca744..cf0b4ace5bf9 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -220,7 +220,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
>
> if (flags & MAP_FIXED) {
> /* Ok, don't mess with it. */
> - return mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
> + return current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
> }
> flags &= ~MAP_SHARED;
>
> @@ -233,8 +233,9 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
> align_goal = (64UL * 1024);
>
> do {
> - addr = mm_get_unmapped_area(current->mm, NULL, orig_addr,
> - len + (align_goal - PAGE_SIZE), pgoff, flags);
> + addr = current_get_unmapped_area(NULL, orig_addr,
> + len + (align_goal - PAGE_SIZE),
> + pgoff, flags);
> if (!(addr & ~PAGE_MASK)) {
> addr = (addr + (align_goal - 1UL)) & ~(align_goal - 1UL);
> break;
> @@ -252,7 +253,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
> * be obtained.
> */
> if (addr & ~PAGE_MASK)
> - addr = mm_get_unmapped_area(current->mm, NULL, orig_addr, len, pgoff, flags);
> + addr = current_get_unmapped_area(NULL, orig_addr, len, pgoff, flags);
>
> return addr;
> }
> diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
> index 22b65a5f5ec6..5f7bfd9035f7 100644
> --- a/arch/x86/kernel/cpu/sgx/driver.c
> +++ b/arch/x86/kernel/cpu/sgx/driver.c
> @@ -113,7 +113,7 @@ static unsigned long sgx_get_unmapped_area(struct file *file,
> if (flags & MAP_FIXED)
> return addr;
>
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
>
> #ifdef CONFIG_COMPAT
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 7c359cc406d5..a29c4bd506d5 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -546,7 +546,7 @@ static unsigned long get_unmapped_area_zero(struct file *file,
> }
>
> /* Otherwise flags & MAP_PRIVATE: with no shmem object beneath it */
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> #else
> return -ENOSYS;
> #endif
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index eb61598247a9..c379902307b7 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -329,14 +329,14 @@ static unsigned long dax_get_unmapped_area(struct file *filp,
> if ((off + len_align) < off)
> goto out;
>
> - addr_align = mm_get_unmapped_area(current->mm, filp, addr, len_align,
> - pgoff, flags);
> + addr_align = current_get_unmapped_area(filp, addr, len_align,
> + pgoff, flags);
> if (!IS_ERR_VALUE(addr_align)) {
> addr_align += (off - addr_align) & (align - 1);
> return addr_align;
> }
> out:
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
>
> static const struct address_space_operations dev_dax_aops = {
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index d19434e2a58e..24a6aeac3de5 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -455,7 +455,7 @@ pde_get_unmapped_area(struct proc_dir_entry *pde, struct file *file, unsigned lo
> return pde->proc_ops->proc_get_unmapped_area(file, orig_addr, len, pgoff, flags);
>
> #ifdef CONFIG_MMU
> - return mm_get_unmapped_area(current->mm, file, orig_addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, orig_addr, len, pgoff, flags);
> #endif
>
> return orig_addr;
> diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
> index b45c7edc3225..85f57de31102 100644
> --- a/fs/ramfs/file-mmu.c
> +++ b/fs/ramfs/file-mmu.c
> @@ -35,7 +35,7 @@ static unsigned long ramfs_mmu_get_unmapped_area(struct file *file,
> unsigned long addr, unsigned long len, unsigned long pgoff,
> unsigned long flags)
> {
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
>
> const struct file_operations ramfs_file_operations = {
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 91546493c43d..c67c7de05c7a 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -187,9 +187,9 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags);
>
> -unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
> - unsigned long addr, unsigned long len,
> - unsigned long pgoff, unsigned long flags);
> +unsigned long current_get_unmapped_area(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags);
>
> unsigned long
> arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
> diff --git a/io_uring/memmap.c b/io_uring/memmap.c
> index 4785d6af5fee..1aaea32c797c 100644
> --- a/io_uring/memmap.c
> +++ b/io_uring/memmap.c
> @@ -305,7 +305,7 @@ unsigned long io_uring_get_unmapped_area(struct file *filp, unsigned long addr,
> #else
> addr = 0UL;
> #endif
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
>
> #else /* !CONFIG_MMU */
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 4a1be699bb82..054486f7c453 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -314,7 +314,7 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
> return -EINVAL;
> }
>
> - ret = mm_get_unmapped_area(current->mm, filp, addr, len * 2, 0, flags);
> + ret = current_get_unmapped_area(filp, addr, len * 2, 0, flags);
> if (IS_ERR_VALUE(ret))
> return ret;
> if ((ret >> 32) == ((ret + len - 1) >> 32))
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2222c3ff88e7..d9ff2843f6ef 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -992,7 +992,7 @@ static unsigned long bpf_get_unmapped_area(struct file *filp, unsigned long addr
> if (map->ops->map_get_unmapped_area)
> return map->ops->map_get_unmapped_area(filp, addr, len, pgoff, flags);
> #ifdef CONFIG_MMU
> - return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
> + return current_get_unmapped_area(filp, addr, len, pgoff, flags);
> #else
> return addr;
> #endif
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 83b4682ec85c..4e98a907c53d 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1901,16 +1901,15 @@ __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
> return error ? error : addr;
> }
>
> -unsigned long
> -mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
> - unsigned long addr, unsigned long len,
> - unsigned long pgoff, unsigned long flags)
> +unsigned long current_get_unmapped_area(struct file *file, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags)
> {
> - if (test_bit(MMF_TOPDOWN, &mm->flags))
> + if (test_bit(MMF_TOPDOWN, ¤t->mm->flags))
> return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
> return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> }
> -EXPORT_SYMBOL(mm_get_unmapped_area);
> +EXPORT_SYMBOL(current_get_unmapped_area);
>
> /**
> * find_vma_intersection() - Look up the first VMA which intersects the interval
> diff --git a/mm/shmem.c b/mm/shmem.c
> index f5d60436b604..c0acd7db93c8 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2276,8 +2276,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> if (len > TASK_SIZE)
> return -ENOMEM;
>
> - addr = mm_get_unmapped_area(current->mm, file, uaddr, len, pgoff,
> - flags);
> + addr = current_get_unmapped_area(file, uaddr, len, pgoff, flags);
>
> if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> return addr;
> @@ -2334,8 +2333,8 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> if (inflated_len < len)
> return addr;
>
> - inflated_addr = mm_get_unmapped_area(current->mm, NULL, uaddr,
> - inflated_len, 0, flags);
> + inflated_addr = current_get_unmapped_area(NULL, uaddr,
> + inflated_len, 0, flags);
> if (IS_ERR_VALUE(inflated_addr))
> return addr;
> if (inflated_addr & ~PAGE_MASK)
> @@ -4799,7 +4798,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags)
> {
> - return mm_get_unmapped_area(current->mm, file, addr, len, pgoff, flags);
> + return current_get_unmapped_area(file, addr, len, pgoff, flags);
> }
> #endif
>
>
> base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
Reviewed-by: Jarkko Sakkinen <[email protected]>
BR, Jarkko