Re: [PATCH mm-unstable v2 05/16] mm: arc: Use generic_mmap_hint()

* Kalesh Singh <[email protected]> [241211 18:28]:
> Introduce arc arch_mmap_hint() and define HAVE_ARCH_MMAP_HINT.
> This is a preparatory patch, no functional change is introduced.

You have changed the order of the map fixed check and the len check.

I *think* what you have done is correct, but your comment is certainly
wrong.

Your generic call also has more checks than exist in this version of the
code - which, again is probably good, but a functional change surely?

> 
> Signed-off-by: Kalesh Singh <[email protected]>
> ---
> 
> Changes in v2:
>   - MAP_FIXED case is also handled in arch_mmap_hint() since this is just a
>     special case of the hint addr being "enforced", per Yang Shi.
>   - Consolidate error handling in arch_mmap_hint().
> 
>  arch/arc/include/asm/pgtable.h |  1 +
>  arch/arc/mm/mmap.c             | 43 +++++++++++++++++-----------------
>  2 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
> index 4cf45a99fd79..af3210ea4888 100644
> --- a/arch/arc/include/asm/pgtable.h
> +++ b/arch/arc/include/asm/pgtable.h
> @@ -28,6 +28,7 @@ extern pgd_t swapper_pg_dir[] __aligned(PAGE_SIZE);
>  
>  /* to cope with aliasing VIPT cache */
>  #define HAVE_ARCH_UNMAPPED_AREA
> +#define HAVE_ARCH_MMAP_HINT
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/arc/mm/mmap.c b/arch/arc/mm/mmap.c
> index 2185afe8d59f..df01d4d9964b 100644
> --- a/arch/arc/mm/mmap.c
> +++ b/arch/arc/mm/mmap.c
> @@ -14,6 +14,26 @@
>  
>  #include <asm/cacheflush.h>
>  
> +unsigned long arch_mmap_hint(struct file *filp, unsigned long addr,
> +			     unsigned long len, unsigned long pgoff,
> +			     unsigned long flags)
> +{
> +	if (len > TASK_SIZE)
> +		return -ENOMEM;
> +
> +	/*
> +	 * We enforce the MAP_FIXED case.
> +	 */
> +	if (flags & MAP_FIXED) {
> +		if (flags & MAP_SHARED &&
> +		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
> +			return -EINVAL;
> +		return addr;
> +	}
> +
> +	return generic_mmap_hint(filp, addr, len, pgoff, flags);
> +}
> +
>  /*
>   * Ensure that shared mappings are correctly aligned to
>   * avoid aliasing issues with VIPT caches.
> @@ -27,30 +47,11 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  		unsigned long flags, vm_flags_t vm_flags)
>  {
>  	struct mm_struct *mm = current->mm;
> -	struct vm_area_struct *vma;
>  	struct vm_unmapped_area_info info = {};
>  
> -	/*
> -	 * We enforce the MAP_FIXED case.
> -	 */
> -	if (flags & MAP_FIXED) {
> -		if (flags & MAP_SHARED &&
> -		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
> -			return -EINVAL;
> +	addr = arch_mmap_hint(filp, addr, len, pgoff, flags);
> +	if (addr)
>  		return addr;
> -	}
> -
> -	if (len > TASK_SIZE)
> -		return -ENOMEM;
> -
> -	if (addr) {
> -		addr = PAGE_ALIGN(addr);
> -
> -		vma = find_vma(mm, addr);
> -		if (TASK_SIZE - len >= addr &&
> -		    (!vma || addr + len <= vm_start_gap(vma)))
> -			return addr;
> -	}
>  
>  	info.length = len;
>  	info.low_limit = mm->mmap_base;
> -- 
> 2.47.0.338.g60cca15819-goog
> 
>