From: Charlie Jenkins <charlie@rivosinc•com>
To: Lorenzo Stoakes <lorenzo.stoakes@oracle•com>
Cc: Arnd Bergmann <arnd@arndb•de>,
Paul Walmsley <paul.walmsley@sifive•com>,
Palmer Dabbelt <palmer@dabbelt•com>,
Albert Ou <aou@eecs•berkeley.edu>,
Catalin Marinas <catalin.marinas@arm•com>,
Will Deacon <will@kernel•org>,
Michael Ellerman <mpe@ellerman•id.au>,
Nicholas Piggin <npiggin@gmail•com>,
Christophe Leroy <christophe.leroy@csgroup•eu>,
Naveen N Rao <naveen@kernel•org>,
Muchun Song <muchun.song@linux•dev>,
Andrew Morton <akpm@linux-foundation•org>,
"Liam R. Howlett" <Liam.Howlett@oracle•com>,
Vlastimil Babka <vbabka@suse•cz>,
Thomas Gleixner <tglx@linutronix•de>,
Ingo Molnar <mingo@redhat•com>, Borislav Petkov <bp@alien8•de>,
Dave Hansen <dave.hansen@linux•intel.com>,
x86@kernel•org, "H. Peter Anvin" <hpa@zytor•com>,
Huacai Chen <chenhuacai@kernel•org>,
WANG Xuerui <kernel@xen0n•name>,
Russell King <linux@armlinux•org.uk>,
Thomas Bogendoerfer <tsbogend@alpha•franken.de>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership•com>,
Helge Deller <deller@gmx•de>,
Alexander Gordeev <agordeev@linux•ibm.com>,
Gerald Schaefer <gerald.schaefer@linux•ibm.com>,
Heiko Carstens <hca@linux•ibm.com>,
Vasily Gorbik <gor@linux•ibm.com>,
Christian Borntraeger <borntraeger@linux•ibm.com>,
Sven Schnelle <svens@linux•ibm.com>,
Yoshinori Sato <ysato@users•sourceforge.jp>,
Rich Felker <dalias@libc•org>,
John Paul Adrian Glaubitz <glaubitz@physik•fu-berlin.de>,
"David S. Miller" <davem@davemloft•net>,
Andreas Larsson <andreas@gaisler•com>,
Shuah Khan <shuah@kernel•org>,
Alexandre Ghiti <alexghiti@rivosinc•com>,
linux-arch@vger•kernel.org, linux-kernel@vger•kernel.org,
Palmer Dabbelt <palmer@rivosinc•com>,
linux-riscv@lists•infradead.org,
linux-arm-kernel@lists•infradead.org,
linuxppc-dev@lists•ozlabs.org, linux-mm@kvack•org,
loongarch@lists•linux.dev, linux-mips@vger•kernel.org,
linux-parisc@vger•kernel.org, linux-s390@vger•kernel.org,
linux-sh@vger•kernel.org, sparclinux@vger•kernel.org,
linux-kselftest@vger•kernel.org
Subject: Re: [PATCH 16/16] selftests/mm: Create MAP_BELOW_HINT test
Date: Wed, 28 Aug 2024 11:13:11 -0700 [thread overview]
Message-ID: <Zs9oty09IysKUHCF@ghost> (raw)
In-Reply-To: <71528354-4884-41e4-a4a9-318e26187f86@lucifer.local>
On Wed, Aug 28, 2024 at 06:48:33PM +0100, Lorenzo Stoakes wrote:
> On Tue, Aug 27, 2024 at 10:49:22PM GMT, Charlie Jenkins wrote:
> > Add a selftest for MAP_BELOW_HINT that maps until it runs out of space
> > below the hint address.
> >
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc•com>
> > ---
> > tools/testing/selftests/mm/Makefile | 1 +
> > tools/testing/selftests/mm/map_below_hint.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> > index cfad627e8d94..4e2de85267b5 100644
> > --- a/tools/testing/selftests/mm/Makefile
> > +++ b/tools/testing/selftests/mm/Makefile
> > @@ -50,6 +50,7 @@ TEST_GEN_FILES += hugepage-shm
> > TEST_GEN_FILES += hugepage-vmemmap
> > TEST_GEN_FILES += khugepaged
> > TEST_GEN_FILES += madv_populate
> > +TEST_GEN_FILES += map_below_hint
> > TEST_GEN_FILES += map_fixed_noreplace
> > TEST_GEN_FILES += map_hugetlb
> > TEST_GEN_FILES += map_populate
> > diff --git a/tools/testing/selftests/mm/map_below_hint.c b/tools/testing/selftests/mm/map_below_hint.c
> > new file mode 100644
> > index 000000000000..305274c5af49
> > --- /dev/null
> > +++ b/tools/testing/selftests/mm/map_below_hint.c
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Test the MAP_BELOW_HINT mmap flag.
> > + */
> > +#include <sys/mman.h>
> > +#include "../kselftest.h"
> > +
> > +#define ADDR 0x1000000UL
> > +#define LENGTH (ADDR / 100)
> > +
> > +#define MAP_BELOW_HINT 0x8000000 /* Not defined in all libc */
> > +
> > +/*
> > + * Map memory with MAP_BELOW_HINT until no memory left. Ensure that all returned
> > + * addresses are below the hint.
> > + */
> > +int main(int argc, char **argv)
> > +{
> > + void *addr;
> > +
> > + do {
> > + addr = mmap((void *)ADDR, LENGTH, MAP_ANONYMOUS, MAP_BELOW_HINT, -1, 0);
>
> How can this be correct? mmap() has parameters:
>
> void *mmap(void addr[.length], size_t length, int prot, int flags,
> int fd, off_t offset);
>
> You'r setting prot = MAP_ANONYMOUS, flags = MAP_BELOW_HINT...
>
> This surely should be:
>
> mmap(..., PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE | MAP_BELOW_HINT, -1, 0);
>
> > + } while (addr == MAP_FAILED && (unsigned long)addr <= ADDR);
>
> How can addr == MAP_FAILED (i.e. ~0) and addr <= ADDR? This will just loop
> through once...
>
> If you want to make sure you're getting mappings only below the hint until
> you start getting MAP_FAILED's you'll need to handle this more robustly.
>
> > +
> > + if (addr != MAP_FAILED && (unsigned long)addr > ADDR)
> > + ksft_exit_fail_msg("mmap returned address above hint with MAP_BELOW_HINT\n");
>
> This is just going to fail because your flags are wrong then wrongly claim
> to have passed...
I obviously didn't spend enough time thinking about this test case...
You are correct that I wrote this incorrectly. I will make a proper test case
and send out a new version.
- Charlie
>
> > +
> > + ksft_test_result_pass("MAP_BELOW_HINT works\n");
> > +}
> >
> > --
> > 2.45.0
> >
next prev parent reply other threads:[~2024-08-28 18:13 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-28 5:49 [PATCH 00/16] mm: Introduce MAP_BELOW_HINT Charlie Jenkins
2024-08-28 5:49 ` [PATCH 01/16] mm: Add MAP_BELOW_HINT Charlie Jenkins
2024-08-28 5:49 ` [PATCH 02/16] riscv: mm: Do not restrict mmap address based on hint Charlie Jenkins
2024-08-28 5:49 ` [PATCH 03/16] mm: Add flag and len param to arch_get_mmap_base() Charlie Jenkins
2024-08-28 5:49 ` [PATCH 04/16] mm: Add generic MAP_BELOW_HINT Charlie Jenkins
2024-08-28 5:49 ` [PATCH 05/16] riscv: mm: Support MAP_BELOW_HINT Charlie Jenkins
2024-08-28 5:49 ` [PATCH 06/16] arm64: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 07/16] powerpc: " Charlie Jenkins
2024-08-28 6:34 ` Christophe Leroy
2024-08-28 17:29 ` Charlie Jenkins
2024-08-31 16:30 ` David Laight
2024-08-28 5:49 ` [PATCH 08/16] x86: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 09/16] loongarch: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 10/16] arm: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 11/16] mips: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 12/16] parisc: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 13/16] s390: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 14/16] sh: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 15/16] sparc: " Charlie Jenkins
2024-08-28 5:49 ` [PATCH 16/16] selftests/mm: Create MAP_BELOW_HINT test Charlie Jenkins
2024-08-28 17:48 ` Lorenzo Stoakes
2024-08-28 18:13 ` Charlie Jenkins [this message]
2024-08-28 18:19 ` [PATCH 00/16] mm: Introduce MAP_BELOW_HINT Lorenzo Stoakes
2024-08-29 7:14 ` Charlie Jenkins
2024-08-28 18:29 ` Dave Hansen
2024-08-28 20:15 ` Charlie Jenkins
2024-08-29 16:54 ` Dave Hansen
2024-08-29 19:36 ` Liam R. Howlett
2024-08-30 1:10 ` Charlie Jenkins
2024-08-30 1:00 ` Charlie Jenkins
2024-08-28 18:31 ` Liam R. Howlett
2024-08-28 20:59 ` Charlie Jenkins
2024-08-28 21:39 ` Charlie Jenkins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zs9oty09IysKUHCF@ghost \
--to=charlie@rivosinc$(echo .)com \
--cc=James.Bottomley@hansenpartnership$(echo .)com \
--cc=Liam.Howlett@oracle$(echo .)com \
--cc=agordeev@linux$(echo .)ibm.com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=alexghiti@rivosinc$(echo .)com \
--cc=andreas@gaisler$(echo .)com \
--cc=aou@eecs$(echo .)berkeley.edu \
--cc=arnd@arndb$(echo .)de \
--cc=borntraeger@linux$(echo .)ibm.com \
--cc=bp@alien8$(echo .)de \
--cc=catalin.marinas@arm$(echo .)com \
--cc=chenhuacai@kernel$(echo .)org \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=dalias@libc$(echo .)org \
--cc=dave.hansen@linux$(echo .)intel.com \
--cc=davem@davemloft$(echo .)net \
--cc=deller@gmx$(echo .)de \
--cc=gerald.schaefer@linux$(echo .)ibm.com \
--cc=glaubitz@physik$(echo .)fu-berlin.de \
--cc=gor@linux$(echo .)ibm.com \
--cc=hca@linux$(echo .)ibm.com \
--cc=hpa@zytor$(echo .)com \
--cc=kernel@xen0n$(echo .)name \
--cc=linux-arch@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-kselftest@vger$(echo .)kernel.org \
--cc=linux-mips@vger$(echo .)kernel.org \
--cc=linux-mm@kvack$(echo .)org \
--cc=linux-parisc@vger$(echo .)kernel.org \
--cc=linux-riscv@lists$(echo .)infradead.org \
--cc=linux-s390@vger$(echo .)kernel.org \
--cc=linux-sh@vger$(echo .)kernel.org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=loongarch@lists$(echo .)linux.dev \
--cc=lorenzo.stoakes@oracle$(echo .)com \
--cc=mingo@redhat$(echo .)com \
--cc=mpe@ellerman$(echo .)id.au \
--cc=muchun.song@linux$(echo .)dev \
--cc=naveen@kernel$(echo .)org \
--cc=npiggin@gmail$(echo .)com \
--cc=palmer@dabbelt$(echo .)com \
--cc=palmer@rivosinc$(echo .)com \
--cc=paul.walmsley@sifive$(echo .)com \
--cc=shuah@kernel$(echo .)org \
--cc=sparclinux@vger$(echo .)kernel.org \
--cc=svens@linux$(echo .)ibm.com \
--cc=tglx@linutronix$(echo .)de \
--cc=tsbogend@alpha$(echo .)franken.de \
--cc=vbabka@suse$(echo .)cz \
--cc=will@kernel$(echo .)org \
--cc=x86@kernel$(echo .)org \
--cc=ysato@users$(echo .)sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox