[LTP] [PATCH 04/13] Hugetlb: Migrating libhugetlbfs mprotect
Li Wang
liwang@redhat.com
Tue Dec 27 06:48:46 CET 2022
Hi Tarun,
This test itself is no problem but looks a bit messy in code organization.
At least we need to do some refactored work to improve the readability and
reduce the (long-term) maintenance workload.
Tarun Sahu <tsahu@linux.ibm.com> wrote:
Migrating the libhugetlbfs/testcases/mprotect.c test
>
> Test Description: This test uses mprotect to change protection of
> hugepage mapping and perform read/write operation. It checks if the
> operation results in expected behaviour as per the protection.
>
> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
> runtest/hugetlb | 1 +
> testcases/kernel/mem/.gitignore | 1 +
> .../kernel/mem/hugetlb/hugemmap/hugemmap23.c | 244 ++++++++++++++++++
> 3 files changed, 246 insertions(+)
> create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap23.c
>
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 8e80db140..8ade3c9ec 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -24,6 +24,7 @@ hugemmap19 hugemmap19
> hugemmap20 hugemmap20
> hugemmap21 hugemmap21
> hugemmap22 hugemmap22
> +hugemmap23 hugemmap23
> hugemmap05_1 hugemmap05 -m
> hugemmap05_2 hugemmap05 -s
> hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore
> b/testcases/kernel/mem/.gitignore
> index 0fd01dbce..ffd831f2e 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -23,6 +23,7 @@
> /hugetlb/hugemmap/hugemmap20
> /hugetlb/hugemmap/hugemmap21
> /hugetlb/hugemmap/hugemmap22
> +/hugetlb/hugemmap/hugemmap23
> /hugetlb/hugeshmat/hugeshmat01
> /hugetlb/hugeshmat/hugeshmat02
> /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap23.c
> b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap23.c
> new file mode 100644
> index 000000000..687b192ae
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap23.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + * Copyright (C) 2005-2006 IBM Corporation.
> + * Author: David Gibson & Adam Litke
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test uses mprotect to change protection of hugepage mapping and
> + * perform read/write operation. It checks if the operation results in
> + * expected behaviour as per the protection.
> + */
> +
> +#include <stdio.h>
> +#include <sys/mount.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <setjmp.h>
> +#include <signal.h>
> +
> +#include "hugetlb.h"
> +
> +#ifndef barrier
> +# ifdef mb
> + /* Redefining the mb() */
> +# define barrier() mb()
> +# else
> +# define barrier() __asm__ __volatile__ ("" : : : "memory")
> +# endif
> +#endif
> +
> +#define MNTPOINT "hugetlbfs/"
> +#define RANDOM_CONSTANT 0x1234ABCD
>
Avoid mixing tab and spaces in the macro definition.
> +
> +static int fd = -1;
>
^ one space is enough.
> +static sigjmp_buf sig_escape;
> +static void *sig_expected = MAP_FAILED;
> +static long hpage_size;
> +
> +static void sig_handler(int signum, siginfo_t *si, void *uc)
> +{
> + (void)uc;
> + if (signum == SIGSEGV) {
> + tst_res(TINFO, "SIGSEGV at %p (sig_expected=%p)",
> si->si_addr,
> + sig_expected);
> + if (si->si_addr == sig_expected)
> + siglongjmp(sig_escape, 1);
> + tst_res(TFAIL, "SIGSEGV somewhere unexpected");
> + } else
> + tst_res(TFAIL, "Unexpected signal %s", strsignal(signum));
> +}
> +
> +static int test_read(void *p)
> +{
> + volatile unsigned long *pl = p;
> + unsigned long x;
> +
> + if (sigsetjmp(sig_escape, 1)) {
> + /* We got a SEGV */
> + sig_expected = MAP_FAILED;
> + return -1;
> + }
> +
> + sig_expected = p;
> + barrier();
> + x = *pl;
> + tst_res(TINFO, "Read back %lu", x);
> + barrier();
> + sig_expected = MAP_FAILED;
> + return 0;
> +}
> +
> +static int test_write(void *p, unsigned long val)
> +{
> + volatile unsigned long *pl = p;
> + unsigned long x;
> +
> + if (sigsetjmp(sig_escape, 1)) {
> + /* We got a SEGV */
> + sig_expected = MAP_FAILED;
> + return -1;
> + }
> +
> + sig_expected = p;
> + barrier();
> + *pl = val;
> + x = *pl;
> + barrier();
> + sig_expected = MAP_FAILED;
> +
> + return (x != val);
> +}
> +
> +static int test_prot(void *p, int prot, char *prot_str)
> +{
> + int r, w;
> +
> + r = test_read(p);
> + tst_res(TINFO, "On Read: %d", r);
> + w = test_write(p, RANDOM_CONSTANT);
> + tst_res(TINFO, "On Write: %d", w);
> +
> + if (prot & PROT_READ) {
> + if (r != 0) {
> + tst_res(TFAIL, "read failed on mmap(prot %s)",
> prot_str);
> + return -1;
> + }
> +
> + } else {
> + if (r != -1) {
> + tst_res(TFAIL, "read succeeded on mmap(prot %s)",
> prot_str);
> + return -1;
> + }
> + }
> +
> + if (prot & PROT_WRITE) {
> + switch (w) {
> + case -1:
> + tst_res(TFAIL, "write failed on mmap(prot %s)",
> prot_str);
> + return -1;
> + case 0:
> + break;
> + case 1:
> + tst_res(TFAIL, "write mismatch on mmap(prot %s)",
> prot_str);
> + return -1;
> + default:
> + tst_res(TWARN, "Bug in test");
> + return -1;
> + }
> + } else {
> + switch (w) {
> + case -1:
> + break;
> + case 0:
> + tst_res(TFAIL, "write succeeded on mmap(prot %s)",
> prot_str);
> + return -1;
> + case 1:
> + tst_res(TFAIL, "write mismatch on mmap(prot %s)",
> prot_str);
> + return -1;
> + default:
> + tst_res(TWARN, "Bug in test");
> + break;
> + }
> + }
> + return 0;
> +}
> +
> +static int test_mprotect(int fd, char *testname,
> + unsigned long len1, int prot1, char *prot1_str,
> + unsigned long len2, int prot2, char *prot2_str)
> +{
> + void *p;
> + int ret;
> +
> + tst_res(TINFO, "Testing %s", testname);
> + tst_res(TINFO, "Mapping with prot %s", prot1_str);
> + p = SAFE_MMAP(NULL, len1, prot1, MAP_SHARED, fd, 0);
> +
> + ret = test_prot(p, prot1, prot1_str);
> + if (ret)
> + goto cleanup;
> + tst_res(TINFO, "mprotect()ing to prot %s", prot2_str);
> + ret = mprotect(p, len2, prot2);
> + if (ret != 0) {
> + tst_res(TFAIL|TERRNO, "%s: mprotect(prot %s)", testname,
> prot2_str);
> + goto cleanup;
> + }
> +
> + ret = test_prot(p, prot2, prot2_str);
> + if (ret)
> + goto cleanup;
> + if (len2 < len1)
> + ret = test_prot(p + len2, prot1, prot1_str);
> +
> +cleanup:
> + SAFE_MUNMAP(p, len1);
> + return ret;
> +}
> +
> +static void run_test(void)
> +{
> + void *p;
> +
>
> + struct sigaction sa = {
> + .sa_sigaction = sig_handler,
> + .sa_flags = SA_SIGINFO,
> + };
> +
> + SAFE_SIGACTION(SIGSEGV, &sa, NULL);
> +
> + fd = tst_creat_unlinked(MNTPOINT, 0);
> + p = SAFE_MMAP(NULL, 2*hpage_size, PROT_READ|PROT_WRITE,
> MAP_SHARED, fd, 0);
> + memset(p, 0, hpage_size);
> + SAFE_MUNMAP(p, hpage_size);
>
I guess moving those preparation work into setup() is better.
> +
> + if (test_mprotect(fd, "R->RW", hpage_size, PROT_READ, "PROT_READ",
> + hpage_size, PROT_READ|PROT_WRITE,
> "PROT_READ|PROT_WRITE"))
> + goto cleanup;
> + if (test_mprotect(fd, "RW->R", hpage_size, PROT_READ|PROT_WRITE,
> + "PROT_READ|PROT_WRITE", hpage_size, PROT_READ,
> "PROT_READ"))
> + goto cleanup;
> +
> + if (test_mprotect(fd, "R->RW 1/2", 2*hpage_size, PROT_READ,
> "PROT_READ",
> + hpage_size, PROT_READ|PROT_WRITE,
> "PROT_READ|PROT_WRITE"))
> + goto cleanup;
> + if (test_mprotect(fd, "RW->R 1/2", 2*hpage_size,
> PROT_READ|PROT_WRITE,
> + "PROT_READ|PROT_WRITE", hpage_size, PROT_READ,
> "PROT_READ"))
> + goto cleanup;
> +
> + if (test_mprotect(fd, "NONE->R", hpage_size, PROT_NONE,
> "PROT_NONE",
> + hpage_size, PROT_READ, "PROT_READ"))
> + goto cleanup;
> + if (test_mprotect(fd, "NONE->RW", hpage_size, PROT_NONE,
> "PROT_NONE",
> + hpage_size, PROT_READ|PROT_WRITE,
> "PROT_READ|PROT_WRITE"))
> + goto cleanup;
>
Can we reorg those test options just into one structure for reducing
the duplicated code? Something maybe like:
static struct mprotect_param {
char *tname;
int pg_num;
int prot1;
char *prot1_str;
int prot2;
char *prot2_str;
} mprotect_params[] = {
{"R->RW", 1,
PROT_READ, "PROT_READ",
PROT_READ|PROT_WRITE, "PROT_READ|PROT_WRITE"},
{"RW->R", 1,
PROT_READ|PROT_WRITE, "PROT_READ|PROT_WRITE",
PROT_READ, "PROT_READ"},
{"R->RW 1/2", 2,
PROT_READ, "PROT_READ",
PROT_READ|PROT_WRITE, "PROT_READ|PROT_WRITE"},
{"RW->R 1/2", 2,
PROT_READ|PROT_WRITE, "PROT_READ|PROT_WRITE",
PROT_READ, "PROT_READ"},
{"NONE->R", 1,
PROT_NONE, "PROT_NONE",
PROT_READ, "PROT_READ"},
{"NONE->RW", 1,
PROT_NONE, "PROT_NONE",
PROT_READ|PROT_WRITE, "PROT_READ|PROT_WRITE"},
};
static void run_test(unsigned int i)
{
struct mprotect_param *mpt = &mprotect_params[i];
if (test_mprotect(fd, mpt->tname,
hpage_size * mpt->pg_num, mpt->prot1,
mpt->prot1_str,
hpage_size, mpt->prot2, mpt->prot2_str))
return;
tst_res(TPASS, "Successfully tested mprotect with hugetlb area\n");
}
static void setup(void)
{
struct sigaction sa = {
.sa_sigaction = sig_handler,
.sa_flags = SA_SIGINFO,
};
SAFE_SIGACTION(SIGSEGV, &sa, NULL);
hpage_size = tst_get_hugepage_size();
fd = tst_creat_unlinked(MNTPOINT, 0);
p = SAFE_MMAP(NULL, 2*hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,
fd, 0);
memset(p, 0, hpage_size);
SAFE_MUNMAP(p, hpage_size);
}
static void cleanup(void)
{
SAFE_MUNMAP(p+hpage_size, hpage_size);
if (fd >= 0)
SAFE_CLOSE(fd);
}
static struct tst_test test = {
.tcnt = ARRAY_SIZE(mprotect_params),
.needs_root = 1,
.mntpoint = MNTPOINT,
.needs_hugetlbfs = 1,
.needs_tmpdir = 1,
.setup = setup,
.cleanup = cleanup,
.test = run_test,
.hugepages = {2, TST_NEEDS},
};
> +
> + tst_res(TPASS, "Successfully tested mprotect with hugetlb area");
>
> +cleanup:
> + SAFE_MUNMAP(p+hpage_size, hpage_size);
> + SAFE_CLOSE(fd);
>
we can move this into cleanup() function as well.
> +}
> +
> +static void setup(void)
> +{
> + hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> +}
> +
> +static void cleanup(void)
> +{
> + if (fd >= 0)
> + SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> + .needs_root = 1,
> + .mntpoint = MNTPOINT,
> + .needs_hugetlbfs = 1,
> + .needs_tmpdir = 1,
> + .setup = setup,
> + .cleanup = cleanup,
> + .test_all = run_test,
> + .hugepages = {2, TST_NEEDS},
> +};
> --
> 2.31.1
>
>
--
Regards,
Li Wang
More information about the ltp
mailing list