Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fe6362b2-c399-7d51-b0e4-15e6ed445767@suse.cz>
Date:   Thu, 16 Jun 2022 10:23:14 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH v4 2/2] [PATCH 2/2] mm: add merging after mremap resize
Content-Language: en-US
To:     =?UTF-8?Q?Jakub_Mat=c4=9bna?= <matenajakub@gmail.com>,
        akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, patches@lists.linux.dev,
        linux-kernel@vger.kernel.org, mhocko@kernel.org,
        mgorman@techsingularity.net, willy@infradead.org,
        liam.howlett@oracle.com, hughd@google.com, kirill@shutemov.name,
        riel@surriel.com, rostedt@goodmis.org, peterz@infradead.org
References: <20220603145719.1012094-1-matenajakub@gmail.com>
 <20220603145719.1012094-3-matenajakub@gmail.com>
From:   Vlastimil Babka <vbabka@suse.cz>
In-Reply-To: <20220603145719.1012094-3-matenajakub@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 6/3/22 16:57, Jakub Matěna wrote:
> When mremap call results in expansion, it might be possible to merge the
> VMA with the next VMA which might become adjacent. This patch adds
> vma_merge call after the expansion is done to try and merge.
> 
> Signed-off-by: Jakub Matěna <matenajakub@gmail.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/mremap.c                              | 19 +++++++++-
>  tools/testing/selftests/vm/mremap_test.c | 47 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 0b93fac76851..66970dcd636a 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/mm.h>
> +#include <linux/mm_inline.h>
>  #include <linux/hugetlb.h>
>  #include <linux/shm.h>
>  #include <linux/ksm.h>
> @@ -23,6 +24,7 @@
>  #include <linux/mmu_notifier.h>
>  #include <linux/uaccess.h>
>  #include <linux/userfaultfd_k.h>
> +#include <linux/mempolicy.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/tlb.h>
> @@ -1014,6 +1016,9 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
>  		/* can we just expand the current mapping? */
>  		if (vma_expandable(vma, new_len - old_len)) {
>  			long pages = (new_len - old_len) >> PAGE_SHIFT;
> +			unsigned long extension_start = addr + old_len;
> +			unsigned long extension_end = addr + new_len;
> +			pgoff_t extension_pgoff = vma->vm_pgoff + (old_len >> PAGE_SHIFT);
>  
>  			if (vma->vm_flags & VM_ACCOUNT) {
>  				if (security_vm_enough_memory_mm(mm, pages)) {
> @@ -1022,8 +1027,18 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
>  				}
>  			}
>  
> -			if (vma_adjust(vma, vma->vm_start, addr + new_len,
> -				       vma->vm_pgoff, NULL)) {
> +			/*
> +			 * Function vma_merge() is called on the extension we are adding to
> +			 * the already existing vma, vma_merge() will merge this extension with
> +			 * the already existing vma (expand operation itself) and possibly also
> +			 * with the next vma if it becomes adjacent to the expanded vma and
> +			 * otherwise compatible.
> +			 */
> +			vma = vma_merge(mm, vma, extension_start, extension_end,
> +					vma->vm_flags, vma->anon_vma, vma->vm_file,
> +					extension_pgoff, vma_policy(vma),
> +					vma->vm_userfaultfd_ctx, anon_vma_name(vma));
> +			if (!vma) {
>  				vm_unacct_memory(pages);
>  				ret = -ENOMEM;
>  				goto out;
> diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
> index db0270127aeb..0865a6cb5bdb 100644
> --- a/tools/testing/selftests/vm/mremap_test.c
> +++ b/tools/testing/selftests/vm/mremap_test.c
> @@ -118,6 +118,48 @@ static unsigned long long get_mmap_min_addr(void)
>  	return addr;
>  }
>  
> +/*
> + * This test validates that merge is called when expanding a mapping.
> + * Mapping containing three pages is created, middle page is unmapped
> + * and then the mapping containing the first page is expanded so that
> + * it fills the created hole. The two parts should merge creating
> + * single mapping with three pages.
> + */
> +static void mremap_expand_merge(unsigned long page_size)
> +{
> +	char *test_name = "mremap expand merge";
> +	FILE *fp;
> +	char *line = NULL;
> +	size_t len = 0;
> +	bool success = false;
> +
> +	char *start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	munmap(start + page_size, page_size);
> +	mremap(start, page_size, 2 * page_size, 0);
> +
> +	fp = fopen("/proc/self/maps", "r");
> +	if (fp == NULL) {
> +		ksft_test_result_fail("%s\n", test_name);
> +		return;
> +	}
> +
> +	while(getline(&line, &len, fp) != -1) {
> +		char *first = strtok(line,"- ");
> +		void *first_val = (void *) strtol(first, NULL, 16);
> +		char *second = strtok(NULL,"- ");
> +		void *second_val = (void *) strtol(second, NULL, 16);
> +		if (first_val == start && second_val == start + 3 * page_size) {
> +			success = true;
> +			break;
> +		}
> +	}
> +	if (success)
> +		ksft_test_result_pass("%s\n", test_name);
> +	else
> +		ksft_test_result_fail("%s\n", test_name);
> +	fclose(fp);
> +}
> +
>  /*
>   * Returns the start address of the mapping on success, else returns
>   * NULL on failure.
> @@ -336,6 +378,7 @@ int main(int argc, char **argv)
>  	int i, run_perf_tests;
>  	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
>  	unsigned int pattern_seed;
> +	int num_expand_tests = 1;
>  	struct test test_cases[MAX_TEST];
>  	struct test perf_test_cases[MAX_PERF_TEST];
>  	int page_size;
> @@ -407,12 +450,14 @@ int main(int argc, char **argv)
>  				(threshold_mb * _1MB >= _1GB);
>  
>  	ksft_set_plan(ARRAY_SIZE(test_cases) + (run_perf_tests ?
> -		      ARRAY_SIZE(perf_test_cases) : 0));
> +		      ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests);
>  
>  	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
>  		run_mremap_test_case(test_cases[i], &failures, threshold_mb,
>  				     pattern_seed);
>  
> +	mremap_expand_merge(page_size);
> +
>  	if (run_perf_tests) {
>  		ksft_print_msg("\n%s\n",
>  		 "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");