diff mbox series

[RESEND,v2,4/4] selftests: vm: add a hugetlb test case

Message ID 20210917034815.80264-5-songmuchun@bytedance.com (mailing list archive)
State New
Headers show
Series Free the 2nd vmemmap page associated with each HugeTLB page | expand

Commit Message

Muchun Song Sept. 17, 2021, 3:48 a.m. UTC 
Since the head vmemmap page frame associated with each HugeTLB page is
reused, we should hide the PG_head flag of tail struct page from the
user. Add a tese case to check whether it is work properly.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 tools/testing/selftests/vm/vmemmap_hugetlb.c | 139 +++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 tools/testing/selftests/vm/vmemmap_hugetlb.c

Comments

Barry Song Sept. 18, 2021, 5:20 a.m. UTC | #1 
On Sat, Sep 18, 2021 at 12:08 AM Muchun Song <songmuchun@bytedance.com> wrote:
>
> Since the head vmemmap page frame associated with each HugeTLB page is
> reused, we should hide the PG_head flag of tail struct page from the
> user. Add a tese case to check whether it is work properly.
>

TBH, I am a bit confused. I was thinking about some kernel unit tests to make
sure those kernel APIs touched by this patchset are still working as before.
This userspace test, while certainly useful for checking the content of page
frames as expected, doesn't directly prove things haven't changed.

In patch 1/4, a couple of APIs have the fixup for the fake head issue.
Do you think a test like the below would be more sensible?
1. alloc 2MB hugeTLB
2. get each page frame
3. apply those APIs in each page frame
4. Those APIs work completely the same as before.

> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  tools/testing/selftests/vm/vmemmap_hugetlb.c | 139 +++++++++++++++++++++++++++
>  1 file changed, 139 insertions(+)
>  create mode 100644 tools/testing/selftests/vm/vmemmap_hugetlb.c
>
> diff --git a/tools/testing/selftests/vm/vmemmap_hugetlb.c b/tools/testing/selftests/vm/vmemmap_hugetlb.c
> new file mode 100644
> index 000000000000..b6e945bf4053
> --- /dev/null
> +++ b/tools/testing/selftests/vm/vmemmap_hugetlb.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * A test case of using hugepage memory in a user application using the
> + * mmap system call with MAP_HUGETLB flag.  Before running this program
> + * make sure the administrator has allocated enough default sized huge
> + * pages to cover the 2 MB allocation.
> + *
> + * For ia64 architecture, Linux kernel reserves Region number 4 for hugepages.
> + * That means the addresses starting with 0x800000... will need to be
> + * specified.  Specifying a fixed address is not required on ppc64, i386
> + * or x86_64.
> + */
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <fcntl.h>
> +
> +#define MAP_LENGTH             (2UL * 1024 * 1024)
> +
> +#ifndef MAP_HUGETLB
> +#define MAP_HUGETLB            0x40000 /* arch specific */
> +#endif
> +
> +#define PAGE_SIZE              4096
> +
> +#define PAGE_COMPOUND_HEAD     (1UL << 15)
> +#define PAGE_COMPOUND_TAIL     (1UL << 16)
> +#define PAGE_HUGE              (1UL << 17)
> +
> +#define HEAD_PAGE_FLAGS                (PAGE_COMPOUND_HEAD | PAGE_HUGE)
> +#define TAIL_PAGE_FLAGS                (PAGE_COMPOUND_TAIL | PAGE_HUGE)
> +
> +#define PM_PFRAME_BITS         55
> +#define PM_PFRAME_MASK         ~((1UL << PM_PFRAME_BITS) - 1)
> +
> +/* Only ia64 requires this */
> +#ifdef __ia64__
> +#define MAP_ADDR               (void *)(0x8000000000000000UL)
> +#define MAP_FLAGS              (MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | MAP_FIXED)
> +#else
> +#define MAP_ADDR               NULL
> +#define MAP_FLAGS              (MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB)
> +#endif
> +
> +static void write_bytes(char *addr, size_t length)
> +{
> +       unsigned long i;
> +
> +       for (i = 0; i < length; i++)
> +               *(addr + i) = (char)i;
> +}
> +
> +static unsigned long virt_to_pfn(void *addr)
> +{
> +       int fd;
> +       unsigned long pagemap;
> +
> +       fd = open("/proc/self/pagemap", O_RDONLY);
> +       if (fd < 0)
> +               return -1UL;
> +
> +       lseek(fd, (unsigned long)addr / PAGE_SIZE * sizeof(pagemap), SEEK_SET);
> +       read(fd, &pagemap, sizeof(pagemap));
> +       close(fd);
> +
> +       return pagemap & ~PM_PFRAME_MASK;
> +}
> +
> +static int check_page_flags(unsigned long pfn)
> +{
> +       int fd, i;
> +       unsigned long pageflags;
> +
> +       fd = open("/proc/kpageflags", O_RDONLY);
> +       if (fd < 0)
> +               return -1;
> +
> +       lseek(fd, pfn * sizeof(pageflags), SEEK_SET);
> +
> +       read(fd, &pageflags, sizeof(pageflags));
> +       if ((pageflags & HEAD_PAGE_FLAGS) != HEAD_PAGE_FLAGS) {
> +               close(fd);
> +               printf("Head page flags (%lx) is invalid\n", pageflags);
> +               return -1;
> +       }
> +
> +       for (i = 1; i < MAP_LENGTH / PAGE_SIZE; i++) {
> +               read(fd, &pageflags, sizeof(pageflags));
> +               if ((pageflags & TAIL_PAGE_FLAGS) != TAIL_PAGE_FLAGS ||
> +                   (pageflags & HEAD_PAGE_FLAGS) == HEAD_PAGE_FLAGS) {
> +                       close(fd);
> +                       printf("Tail page flags (%lx) is invalid\n", pageflags);
> +                       return -1;
> +               }
> +       }
> +
> +       close(fd);
> +
> +       return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +       void *addr;
> +       unsigned long pfn;
> +
> +       addr = mmap(MAP_ADDR, MAP_LENGTH, PROT_READ | PROT_WRITE, MAP_FLAGS, -1, 0);
> +       if (addr == MAP_FAILED) {
> +               perror("mmap");
> +               exit(1);
> +       }
> +
> +       /* Trigger allocation of HugeTLB page. */
> +       write_bytes(addr, MAP_LENGTH);
> +
> +       pfn = virt_to_pfn(addr);
> +       if (pfn == -1UL) {
> +               munmap(addr, MAP_LENGTH);
> +               perror("virt_to_pfn");
> +               exit(1);
> +       }
> +
> +       printf("Returned address is %p whose pfn is %lx\n", addr, pfn);
> +
> +       if (check_page_flags(pfn) < 0) {
> +               munmap(addr, MAP_LENGTH);
> +               perror("check_page_flags");
> +               exit(1);
> +       }
> +
> +       /* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
> +       if (munmap(addr, MAP_LENGTH)) {
> +               perror("munmap");
> +               exit(1);
> +       }
> +
> +       return 0;
> +}
> --
> 2.11.0
>

Thanks
Barry
Muchun Song Sept. 20, 2021, 2:26 p.m. UTC | #2 
On Sat, Sep 18, 2021 at 1:20 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Sat, Sep 18, 2021 at 12:08 AM Muchun Song <songmuchun@bytedance.com> wrote:
> >
> > Since the head vmemmap page frame associated with each HugeTLB page is
> > reused, we should hide the PG_head flag of tail struct page from the
> > user. Add a tese case to check whether it is work properly.
> >
>
> TBH, I am a bit confused. I was thinking about some kernel unit tests to make
> sure those kernel APIs touched by this patchset are still working as before.
> This userspace test, while certainly useful for checking the content of page
> frames as expected, doesn't directly prove things haven't changed.
>
> In patch 1/4, a couple of APIs have the fixup for the fake head issue.
> Do you think a test like the below would be more sensible?
> 1. alloc 2MB hugeTLB

It is done in main().

> 2. get each page frame
> 3. apply those APIs in each page frame
> 4. Those APIs work completely the same as before.

Reading the flags of a page by /proc/kpageflags is done
in stable_page_flags(), which has invoked PageHead(),
PageTail(), PageCompound() and compound_head().
If those APIs work properly, the head page must have
15 and 17 bits set. And tail pages must have 16 and 17
bits set but 15 unset.

So I think check_page_flags() has done the step 2 to 4.
What do you think?

Thanks.
Barry Song Sept. 21, 2021, 12:28 a.m. UTC | #3 
On Tue, Sep 21, 2021 at 2:26 AM Muchun Song <songmuchun@bytedance.com> wrote:
>
> On Sat, Sep 18, 2021 at 1:20 PM Barry Song <21cnbao@gmail.com> wrote:
> >
> > On Sat, Sep 18, 2021 at 12:08 AM Muchun Song <songmuchun@bytedance.com> wrote:
> > >
> > > Since the head vmemmap page frame associated with each HugeTLB page is
> > > reused, we should hide the PG_head flag of tail struct page from the
> > > user. Add a tese case to check whether it is work properly.
> > >
> >
> > TBH, I am a bit confused. I was thinking about some kernel unit tests to make
> > sure those kernel APIs touched by this patchset are still working as before.
> > This userspace test, while certainly useful for checking the content of page
> > frames as expected, doesn't directly prove things haven't changed.
> >
> > In patch 1/4, a couple of APIs have the fixup for the fake head issue.
> > Do you think a test like the below would be more sensible?
> > 1. alloc 2MB hugeTLB
>
> It is done in main().
>
> > 2. get each page frame
> > 3. apply those APIs in each page frame
> > 4. Those APIs work completely the same as before.
>
> Reading the flags of a page by /proc/kpageflags is done
> in stable_page_flags(), which has invoked PageHead(),
> PageTail(), PageCompound() and compound_head().
> If those APIs work properly, the head page must have
> 15 and 17 bits set. And tail pages must have 16 and 17
> bits set but 15 unset.
>
> So I think check_page_flags() has done the step 2 to 4.
> What do you think?

yes. Thanks for your explanation. thereby, I think we just need some doc
here to explain what it is checking. something like
/*
 * pages other than the first page must be tail and shouldn't be head;
 * this also verifies kernel has correctly set the fake page_head to tail
 * while hugetlb_free_vmemmap is enabled
 */
+       for (i = 1; i < MAP_LENGTH / PAGE_SIZE; i++) {
+               read(fd, &pageflags, sizeof(pageflags));
+               if ((pageflags & TAIL_PAGE_FLAGS) != TAIL_PAGE_FLAGS ||
+                   (pageflags & HEAD_PAGE_FLAGS) == HEAD_PAGE_FLAGS) {
+                       close(fd);
+                       printf("Tail page flags (%lx) is invalid\n", pageflags);
+                       return -1;
+               }
+       }
>
> Thanks.

Thanks
barry
Muchun Song Sept. 21, 2021, 1:18 p.m. UTC | #4 
On Tue, Sep 21, 2021 at 8:29 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Tue, Sep 21, 2021 at 2:26 AM Muchun Song <songmuchun@bytedance.com> wrote:
> >
> > On Sat, Sep 18, 2021 at 1:20 PM Barry Song <21cnbao@gmail.com> wrote:
> > >
> > > On Sat, Sep 18, 2021 at 12:08 AM Muchun Song <songmuchun@bytedance.com> wrote:
> > > >
> > > > Since the head vmemmap page frame associated with each HugeTLB page is
> > > > reused, we should hide the PG_head flag of tail struct page from the
> > > > user. Add a tese case to check whether it is work properly.
> > > >
> > >
> > > TBH, I am a bit confused. I was thinking about some kernel unit tests to make
> > > sure those kernel APIs touched by this patchset are still working as before.
> > > This userspace test, while certainly useful for checking the content of page
> > > frames as expected, doesn't directly prove things haven't changed.
> > >
> > > In patch 1/4, a couple of APIs have the fixup for the fake head issue.
> > > Do you think a test like the below would be more sensible?
> > > 1. alloc 2MB hugeTLB
> >
> > It is done in main().
> >
> > > 2. get each page frame
> > > 3. apply those APIs in each page frame
> > > 4. Those APIs work completely the same as before.
> >
> > Reading the flags of a page by /proc/kpageflags is done
> > in stable_page_flags(), which has invoked PageHead(),
> > PageTail(), PageCompound() and compound_head().
> > If those APIs work properly, the head page must have
> > 15 and 17 bits set. And tail pages must have 16 and 17
> > bits set but 15 unset.
> >
> > So I think check_page_flags() has done the step 2 to 4.
> > What do you think?
>
> yes. Thanks for your explanation. thereby, I think we just need some doc
> here to explain what it is checking. something like
> /*
>  * pages other than the first page must be tail and shouldn't be head;
>  * this also verifies kernel has correctly set the fake page_head to tail
>  * while hugetlb_free_vmemmap is enabled
>  */

Got it. Will do. Thanks.

> +       for (i = 1; i < MAP_LENGTH / PAGE_SIZE; i++) {
> +               read(fd, &pageflags, sizeof(pageflags));
> +               if ((pageflags & TAIL_PAGE_FLAGS) != TAIL_PAGE_FLAGS ||
> +                   (pageflags & HEAD_PAGE_FLAGS) == HEAD_PAGE_FLAGS) {
> +                       close(fd);
> +                       printf("Tail page flags (%lx) is invalid\n", pageflags);
> +                       return -1;
> +               }
> +       }
> >
> > Thanks.
>
> Thanks
> barry
diff mbox series

Patch

diff --git a/tools/testing/selftests/vm/vmemmap_hugetlb.c b/tools/testing/selftests/vm/vmemmap_hugetlb.c
new file mode 100644
index 000000000000..b6e945bf4053
--- /dev/null
+++ b/tools/testing/selftests/vm/vmemmap_hugetlb.c
@@ -0,0 +1,139 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A test case of using hugepage memory in a user application using the
+ * mmap system call with MAP_HUGETLB flag.  Before running this program
+ * make sure the administrator has allocated enough default sized huge
+ * pages to cover the 2 MB allocation.
+ *
+ * For ia64 architecture, Linux kernel reserves Region number 4 for hugepages.
+ * That means the addresses starting with 0x800000... will need to be
+ * specified.  Specifying a fixed address is not required on ppc64, i386
+ * or x86_64.
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <fcntl.h>
+
+#define MAP_LENGTH		(2UL * 1024 * 1024)
+
+#ifndef MAP_HUGETLB
+#define MAP_HUGETLB		0x40000	/* arch specific */
+#endif
+
+#define PAGE_SIZE		4096
+
+#define PAGE_COMPOUND_HEAD	(1UL << 15)
+#define PAGE_COMPOUND_TAIL	(1UL << 16)
+#define PAGE_HUGE		(1UL << 17)
+
+#define HEAD_PAGE_FLAGS		(PAGE_COMPOUND_HEAD | PAGE_HUGE)
+#define TAIL_PAGE_FLAGS		(PAGE_COMPOUND_TAIL | PAGE_HUGE)
+
+#define PM_PFRAME_BITS		55
+#define PM_PFRAME_MASK		~((1UL << PM_PFRAME_BITS) - 1)
+
+/* Only ia64 requires this */
+#ifdef __ia64__
+#define MAP_ADDR		(void *)(0x8000000000000000UL)
+#define MAP_FLAGS		(MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | MAP_FIXED)
+#else
+#define MAP_ADDR		NULL
+#define MAP_FLAGS		(MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB)
+#endif
+
+static void write_bytes(char *addr, size_t length)
+{
+	unsigned long i;
+
+	for (i = 0; i < length; i++)
+		*(addr + i) = (char)i;
+}
+
+static unsigned long virt_to_pfn(void *addr)
+{
+	int fd;
+	unsigned long pagemap;
+
+	fd = open("/proc/self/pagemap", O_RDONLY);
+	if (fd < 0)
+		return -1UL;
+
+	lseek(fd, (unsigned long)addr / PAGE_SIZE * sizeof(pagemap), SEEK_SET);
+	read(fd, &pagemap, sizeof(pagemap));
+	close(fd);
+
+	return pagemap & ~PM_PFRAME_MASK;
+}
+
+static int check_page_flags(unsigned long pfn)
+{
+	int fd, i;
+	unsigned long pageflags;
+
+	fd = open("/proc/kpageflags", O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	lseek(fd, pfn * sizeof(pageflags), SEEK_SET);
+
+	read(fd, &pageflags, sizeof(pageflags));
+	if ((pageflags & HEAD_PAGE_FLAGS) != HEAD_PAGE_FLAGS) {
+		close(fd);
+		printf("Head page flags (%lx) is invalid\n", pageflags);
+		return -1;
+	}
+
+	for (i = 1; i < MAP_LENGTH / PAGE_SIZE; i++) {
+		read(fd, &pageflags, sizeof(pageflags));
+		if ((pageflags & TAIL_PAGE_FLAGS) != TAIL_PAGE_FLAGS ||
+		    (pageflags & HEAD_PAGE_FLAGS) == HEAD_PAGE_FLAGS) {
+			close(fd);
+			printf("Tail page flags (%lx) is invalid\n", pageflags);
+			return -1;
+		}
+	}
+
+	close(fd);
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	void *addr;
+	unsigned long pfn;
+
+	addr = mmap(MAP_ADDR, MAP_LENGTH, PROT_READ | PROT_WRITE, MAP_FLAGS, -1, 0);
+	if (addr == MAP_FAILED) {
+		perror("mmap");
+		exit(1);
+	}
+
+	/* Trigger allocation of HugeTLB page. */
+	write_bytes(addr, MAP_LENGTH);
+
+	pfn = virt_to_pfn(addr);
+	if (pfn == -1UL) {
+		munmap(addr, MAP_LENGTH);
+		perror("virt_to_pfn");
+		exit(1);
+	}
+
+	printf("Returned address is %p whose pfn is %lx\n", addr, pfn);
+
+	if (check_page_flags(pfn) < 0) {
+		munmap(addr, MAP_LENGTH);
+		perror("check_page_flags");
+		exit(1);
+	}
+
+	/* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
+	if (munmap(addr, MAP_LENGTH)) {
+		perror("munmap");
+		exit(1);
+	}
+
+	return 0;
+}