From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E592BC282EC for ; Mon, 17 Mar 2025 19:05:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EB812280002; Mon, 17 Mar 2025 15:05:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E6665280001; Mon, 17 Mar 2025 15:05:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D07B4280002; Mon, 17 Mar 2025 15:05:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B6BE8280001 for ; Mon, 17 Mar 2025 15:05:32 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id C7F911C7D21 for ; Mon, 17 Mar 2025 19:05:32 +0000 (UTC) X-FDA: 83231971704.02.FC9BF79 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf27.hostedemail.com (Postfix) with ESMTP id A9F2940015 for ; Mon, 17 Mar 2025 19:05:29 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="i/1GN7Na"; spf=pass (imf27.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1742238329; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=UsOXMl93ebs5LY5H0WA9B3RWYRXifobzHDX+zAABh18=; b=6lfR4roayCKnnrj+7Tv1SPxKV7u8ZYiyXSFNqGaGzwPMuN0iTXGU5yy1SDfpj/NFaTBltU lakzBg4m5pfNmPZe3biUae2mHULunfUQQQxGrgqpbqlqCW7oMA7bZG2hILsTBPIL6CV7sU 0aQ4H9xfr53WT8wJWJAymAn1O6xjQhI= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="i/1GN7Na"; spf=pass (imf27.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1742238329; a=rsa-sha256; cv=none; b=2jH+b0yWSTczmsMHhYphSQVzIWpglCT9jH6i/k0R3Ft9U4KjZJKEZn4zcfdwdrY1TwoMTT xbri6AiIALR3nhghm3FsxSqCzNIhyY8bVNhvS0jg6bdc4xZ27rFIk2o7cndK62QiWclT5g WWcHNEZfSrf6koHtCn4hlh+0numYvcw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1742238328; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UsOXMl93ebs5LY5H0WA9B3RWYRXifobzHDX+zAABh18=; b=i/1GN7Na1gDsleiWMUpdUAK2Q+Ahzn/ruPiqi4R/repamrNyNcpRjhB/xiMn6hfw50/MOc jiFmVt0qNWHlKswK/ChUlQ+AAiPlfRtE3Cpm+ypLlxYKTBAPfSoHzYKW32l054QIp1HlZp EWoh+6tKyvWlxLuEOXPYDQIjLoosJNY= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-13--chqb0RyObKLIupMofYguQ-1; Mon, 17 Mar 2025 15:05:27 -0400 X-MC-Unique: -chqb0RyObKLIupMofYguQ-1 X-Mimecast-MFC-AGG-ID: -chqb0RyObKLIupMofYguQ_1742238327 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7c3b6450ed8so843655585a.3 for ; Mon, 17 Mar 2025 12:05:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742238326; x=1742843126; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=UsOXMl93ebs5LY5H0WA9B3RWYRXifobzHDX+zAABh18=; b=vhv0rMZObK8WavqPypmPvoQ3LWMDw59aF7DVyuHoT46rJO3Vu8wzNxELDFnFdMx8bF 6CitboeM86l55Uo0gR7ai3Y4bdQs/6cd10RZQgEMJkjYMVjil7nNX51uaahm/diBv4OB ZODgnJeE35wdPOKgoRe3f5HpdvtP9Kb0Jc9PQQA0381tTW3JeWbUwAYSHHfgwiGrFMHo wRWur7CaArInJSRB3TMNY7Ab/WjYXOjqAk4akVfm5bQF2L3G0nUJEtQjO/kcbB578fMW lDgHEv9//Mxr6C1QaN4kUiuO/b1IXO52qfnCOLnOjf5Mqw0OwbHDclWmgtt0DZeAUskp Tphg== X-Gm-Message-State: AOJu0Yw66edBDbOG5CSB544Iv0Uzo0EU2NZrIifp/9o+/dYSTXCUcg2m xlcRCg/lOFtss5k9nxHTBC+79t2fwvm7NAAw0bumqMTl69lC/Uu6Mro8itpvBRG9ONPGB6qgw3n 6QU6H+Qr4B9/JwcT0z+kR8Jy4mTU2+B7HKSV5neIPGxHdvvqBwRn07gc9 X-Gm-Gg: ASbGncsj7dZ9LKeB8TrJMXokZDh6UmrnQFdk6D1P0Naxw6GpheI48gIdI2dYaGrsQKO 2BQKtZJQTSAsgvgWSFYQVVYKXAdJBNl8d5d9VnJpDFFKLDq7Uxz7b/7hDMaYcBsodNl5mq8+AZf uFWcoQFbALUOyYel5dITz46iy8QCdXu+H5InM03OpwhGGY9P4CaIV0u7IzGcvkZkFp+aUFXYMJk m27nJTbndzeHX/GWQPxa7aR+IdzMdATsezf6DT+/wGtwfmwdE6Ere0L3U+jdFJ7Rn5HiCNBOKbU sdT/69Q= X-Received: by 2002:a05:6214:4009:b0:6e8:f3b0:fa33 with SMTP id 6a1803df08f44-6eaea9ed9e0mr218201606d6.8.1742238326486; Mon, 17 Mar 2025 12:05:26 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE1K39k7zt2GnQKcbjISXoc98vy6YInkdZ+nO3lU4s/J0+SLaRFyON3JbfudsvJiL7zfoW3lA== X-Received: by 2002:a05:6214:4009:b0:6e8:f3b0:fa33 with SMTP id 6a1803df08f44-6eaea9ed9e0mr218201146d6.8.1742238326084; Mon, 17 Mar 2025 12:05:26 -0700 (PDT) Received: from x1.local ([85.131.185.92]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-476bb63e688sm56564131cf.31.2025.03.17.12.05.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Mar 2025 12:05:25 -0700 (PDT) Date: Mon, 17 Mar 2025 15:05:22 -0400 From: Peter Xu To: Marty Kareem Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, shuah@kernel.org Subject: Re: [PATCH RESEND] mm/selftest: Replace static BASE_PMD_ADDR with dynamic address allocation Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SrV-iB1Spo9t5QOBlMN8CzMzE-ipMYd0wF1hfwppPao_1742238327 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Rspamd-Queue-Id: A9F2940015 X-Rspamd-Server: rspam08 X-Stat-Signature: zptkaimu64h6nbkaos1i7obk1qko4sft X-HE-Tag: 1742238329-829758 X-HE-Meta: U2FsdGVkX19kesz0M1cZgtdV+MjsJqexkVwqCXgYSejywxTrOt65NmXzizVPZrMVfSwf9UiSreIHUhCkausjPduT2NUQHoyYFCe7KW6rvXeRfT/TGtmB9RuH5ejgaJq+/ZPJ72bo9Qq5/WFLxNvkIbpOe7h5FL651wMA1YmYriP3Rl7LH8okKJOr75q1No6FpAm9M4ZkoYo3jETceH/4DnVUBxHRuNnSJQ8S7AQhFPhmfBVID8lGvMnX9q/zELZXx/MnOgjsMiUdIJkmVZ6mnfvtbMMcpAFGSerCmUuw++Ir5ZCB+sDL8ddqGl8oalmZr48Alvzku8qCtRhNYja16pAh6UBqau9tddzEntChff3SAhgjldsz/GWkfqMSqW88H8zl+hZri/VBB617kkdl2QF23hlc84KAj8dZscrnhew1FkGqmvm+FHuZE4PM+ysfnnuh9IFegbjnCIkxAu0t6qosOsQc0+QlFQ/u5c6Z3pQ5YHdLkWa5gMrKT2jgRgIxd7ORcB6ZlB2YgY5exynpgI9kZ1w7L2Ynxu4hdt5ZdtK/dzOIVRCXwxx/HhfG1vNpJ645O+9oKi6oyTN4/rSBpubwr9f71iKuqnV41fvtxSQ9VBvsmoUQ6Iq9NO4TW4tP96Qs2HHu+fdxYHcmBx/PhUEVEXNVnFZ8+euOlfO+fHvY4bbjun35xZaaYi1H8fzGxZLFMT303SrS7iNxfJdBWzzVE44YQF8xwB082A+hSxa+9uZq0RYm3mAw233VU4pXbzG7PXu9NrtJgEAzgZjcjV06v55SmWxitaQOwRSlzIgM0AELCPY2X/OU5a3Oz+tqYOkeQOnG13t1gB/UxWsw4CwCjxYAoKqoPc7HM0+mTjXV94rN5OFTcXnhvakss/X+bACa25BWTq1LDE2IrHYn4PZauCn8iSyUubvkdfKPVG3fh+eIsMrFB5MrWHpEqefaYQtCrIK8I1V03VOQl+6 Iu9mVF6A wVgzhCYjMtSofYbpLwCPflxOiFDP3HDHCN9fi2i3NgFvpp6KcXAoPTD/pvGdJrbSH6OozYpwXbPcH8mn7nmbmDLEItYNoCwECWc/LBJtSHSaIl+eMFv55h9G5r9yzab12Xvc3h6yculK63bQKfZPuQKYabeLlRXZ6ifbawkdZSYhbPxznIExM/rF4inqJvYgz6riloky6dY7/zXg0Ej/er0kqpACJvFlqRHRdeUnoMVgNrGWEyYOc63hzB31esZK049eFgy+p4jkGOhVF0JrNKQU8uJHOriILMzcEYiTvU5lus1/FULS9trz0pVgeCzAZ4IvrqFnBF9GduXtDQTFlYyXP/s/JrJMlmt1caTkWEhiI7muo44stH4xj9AYe3/MJ4FgDIJMkzjK/s+UocUpcNcEmJqRHD+igcyFghbZsavRTLSKtgvyDxDeRzJeRWvQoZRJBRnBK3lTGIdCwxObBEIE9ZqKTc6rvfvNXNh/Rl06N/xHrN4Imz+8kEfKOgcXuA3o7 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hello, Marty, On Thu, Mar 13, 2025 at 10:35:35PM -0400, Marty Kareem wrote: > (RESEND: previous email accidentally sent in HTML format, resending in plain > text) Yes, plan text is better, but when you repost again please send the patch directly instead of making it an attachment. See: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html You could leverage git format-patch or git send-patch. > > This patch addresses a longstanding TODO comment in the userfaultfd tests, > '/linux/tools/testing/selftests/mm/uffd-common.c' > ("/* Use a static addr is ugly */") by replacing hardcoded 1GB addresses > with dynamic allocation. I'd appreciate your review. > > The Problem > ------------ > The current static address approach: > - Causes test failures when other mappings occupy the 1GB region > - Prevents parallel test execution (critical for modern CI/CD systems) > - Breaks on systems with unusual memory layouts Yes, I believe this is a real selftest issue. > > The Solution > ------------ > I implemented a find_suitable_area() helper that: > - Asks the kernel for suggested addresses via mmap(NULL) > - Automatically aligns for huge pages when needed > - Uses MAP_FIXED_NOREPLACE where available (graceful fallback otherwise) > - Adds guard pages between mappings to prevent VMA merging > > Validation > ---------- > I did multiple tests on my implementation to make sure it worked like: > - Multiple parallel test runs > - Memory pressure scenarios > - Edge cases (unusual alignments, sizes, etc.) > - Race conditions and concurrent access > > Performance impact is minimal , about 1.2x overhead compared to the static > approach in benchmarks. > > Why This Matters > ---------------- > - Removes longstanding TODO from the codebase > - Enables safe parallel testing > - Makes tests portable to different environments and memory layouts > - Improves overall test reliability > > This is my first PR for the Linux Kernel and I would be > grateful for your feedback! > > Signed-off-by: MrMartyK > From 5295ee5a7532f1e75364cefa1091dfb49ad320d4 Mon Sep 17 00:00:00 2001 > From: MrMartyK If you want, you may fill this up with your real full name. But it's your call. > Date: Thu, 13 Mar 2025 20:17:43 -0400 > Subject: [PATCH] mm/selftest: Replace static BASE_PMD_ADDR with dynamic > address allocation > > This commit replaces the static BASE_PMD_ADDR in userfaultfd tests with > dynamic address discovery using the find_suitable_area() function. This > addresses a TODO comment in the code which noted that using a static > address was 'ugly'. > > The implementation: > 1. Adds find_suitable_area() that obtains a good address hint from the OS > 2. Updates shmem_allocate_area() to use dynamic allocation > 3. Includes proper fallback mechanisms when preferred addresses unavailable > 4. Works with both MAP_FIXED_NOREPLACE and MAP_FIXED > 5. Maintains backward compatibility with existing tests > > This provides more robust operation when running tests in parallel or in > environments with different memory layouts, while maintaining good > performance (only ~1.2x overhead vs. the static approach). > > Additional updates: > - Added improved code formatting and comments > - Enhanced error handling in fallback cases > - Fixed memory verification in integration tests > > Signed-off-by: MrMartyK > --- > tools/testing/selftests/mm/uffd-common.c | 114 ++++++++++++++++++----- > 1 file changed, 93 insertions(+), 21 deletions(-) > > diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c > index 717539eddf98..b91510da494e 100644 > --- a/tools/testing/selftests/mm/uffd-common.c > +++ b/tools/testing/selftests/mm/uffd-common.c > @@ -7,7 +7,7 @@ > > #include "uffd-common.h" > > -#define BASE_PMD_ADDR ((void *)(1UL << 30)) > +// Removed static BASE_PMD_ADDR definition in favor of dynamic address allocation > > volatile bool test_uffdio_copy_eexist = true; > unsigned long nr_cpus, nr_pages, nr_pages_per_cpu, page_size; > @@ -122,6 +122,39 @@ static void shmem_release_pages(char *rel_area) > err("madvise(MADV_REMOVE) failed"); > } > > +/** > + * Find a suitable virtual address area of the requested size and alignment > + * > + * This function obtains a hint from the OS about where a good place to map > + * memory might be, then aligns it according to the specified alignment > + * requirements. > + * > + * @param size Size of the memory area needed > + * @param alignment Alignment requirement (typically huge page size) > + * @return A suitable address or NULL if none could be found > + */ > +static void *find_suitable_area(size_t size, size_t alignment) > +{ > + void *addr; > + void *hint; > + uintptr_t aligned_addr; > + > + /* First try to get a suggestion from the OS */ > + addr = mmap(NULL, size, PROT_NONE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + if (addr == MAP_FAILED) > + return NULL; > + > + /* Remember the address and unmap it */ > + hint = addr; > + munmap(addr, size); Above PROT_NONE private map trick looks good. But right after munmap(), another thread could take this range away instead using another concurrent mmap(), because the kernel (after munmap() returned here) would think this area free. To make it not possible to happen, IIUC the general way to do this is keeping the pointer without munmap() here, then mmap() with MAP_FIXED the 2nd time directly on top of it, causing atomic unmap of the PROT_NONE range, replacing it with the new mmap() you really need. Before the 2nd mmap(), nothing should be able to race taking that region because in the kernel this range is still occupied. With that, IIUC we also don't need MAP_FIXED_NOREPLACE, because such behavior should exist forever, and it should work on both old/new kernels. > + > + /* Align the address to requested alignment (e.g., huge page size) */ > + aligned_addr = ((uintptr_t)hint + alignment - 1) & ~(alignment - 1); > + > + return (void *)aligned_addr; > +} > + > static int shmem_allocate_area(void **alloc_area, bool is_src) > { > void *area_alias = NULL; > @@ -129,35 +162,74 @@ static int shmem_allocate_area(void **alloc_area, bool is_src) > unsigned long offset = is_src ? 0 : bytes; > char *p = NULL, *p_alias = NULL; > int mem_fd = uffd_mem_fd_create(bytes * 2, false); > + int map_flags; > > - /* TODO: clean this up. Use a static addr is ugly */ > - p = BASE_PMD_ADDR; > - if (!is_src) > + /* Find a suitable address range with appropriate alignment */ > + p = find_suitable_area(bytes, hpage_size); > + if (!p) { > + /* Fallback strategy if finding area fails */ > + fprintf(stderr, "Warning: Could not find suitable address, letting kernel choose\n"); > + } > + > + /* If this is dst area, add offset to prevent overlap with src area */ > + if (!is_src && p) { > + /* Use same spacing as original code to maintain compatibility */ > /* src map + alias + interleaved hpages */ > - p += 2 * (bytes + hpage_size); > - p_alias = p; > - p_alias += bytes; > - p_alias += hpage_size; /* Prevent src/dst VMA merge */ > + p = (char *)p + 2 * (bytes + hpage_size); > + } > > - *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > - mem_fd, offset); > + /* Select flags based on whether we have a preferred address */ > + map_flags = MAP_SHARED; > + if (p) { > +#ifdef MAP_FIXED_NOREPLACE > + map_flags |= MAP_FIXED_NOREPLACE; > +#else > + /* Fallback to regular MAP_FIXED if necessary */ > + map_flags |= MAP_FIXED; > +#endif > + } > + > + /* Try to map at the suggested address, falling back if needed */ > + *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, map_flags, mem_fd, offset); > + > if (*alloc_area == MAP_FAILED) { > - *alloc_area = NULL; > - return -errno; > + /* If fixed mapping failed, try again without it */ > + *alloc_area = mmap(NULL, bytes, PROT_READ | PROT_WRITE, > + MAP_SHARED, mem_fd, offset); > + if (*alloc_area == MAP_FAILED) { > + *alloc_area = NULL; > + close(mem_fd); > + return -errno; > + } > } > - if (*alloc_area != p) > - err("mmap of memfd failed at %p", p); > > - area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED, > - mem_fd, offset); > + /* Calculate a good spot for the alias mapping with space to prevent merging */ > + p_alias = (char *)((uintptr_t)*alloc_area + bytes + hpage_size); > + > + /* Map the alias area */ > + map_flags = MAP_SHARED; > +#ifdef MAP_FIXED_NOREPLACE > + map_flags |= MAP_FIXED_NOREPLACE; > +#else > + map_flags |= MAP_FIXED; > +#endif > + > + area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, > + map_flags, mem_fd, offset); > + > if (area_alias == MAP_FAILED) { > - munmap(*alloc_area, bytes); > - *alloc_area = NULL; > - return -errno; > + /* If fixed mapping failed, try anywhere */ > + area_alias = mmap(NULL, bytes, PROT_READ | PROT_WRITE, > + MAP_SHARED, mem_fd, offset); > + if (area_alias == MAP_FAILED) { > + munmap(*alloc_area, bytes); > + *alloc_area = NULL; > + close(mem_fd); > + return -errno; > + } > } > - if (area_alias != p_alias) > - err("mmap of anonymous memory failed at %p", p_alias); > > + /* Store the alias mapping for later use */ > if (is_src) > area_src_alias = area_alias; > else > -- > 2.43.0 > -- Peter Xu