[LTP] [PATCH v5] Origin: https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/shm-getraw.c

[Cyril Hrubis]

Hi!
> The test creates a shared memory segment, then attaches it to the process’s address space.
> It writes a string to the shared memory from raw device and detaches the shared memory
> segment and finally removes it.
> The purpose of this test is to ensure that the shared memory subsystem is working correctly
> with hugepages. It checks that shared memory segments can be created, attached, written to,
> read from, detached, and removed without errors
> 
> Signed-off-by: Pavithra <pavrampu@linux.vnet.ibm.com>
> ---
>  runtest/hugetlb                               |  2 +
>  testcases/kernel/mem/.gitignore               |  1 +
>  .../kernel/mem/hugetlb/hugeshmmisc/Makefile   |  9 +++
>  .../mem/hugetlb/hugeshmmisc/hugeshmmisc01.c   | 58 +++++++++++++++++++
>  4 files changed, 70 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugeshmmisc/Makefile
>  create mode 100644 testcases/kernel/mem/hugetlb/hugeshmmisc/hugeshmmisc01.c

Maybe we should put the test into testcases/kernel/syscalls/ipc/shmgat/
instead of creating a new directorries in hugetlb.

> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index f294e9aaa..412008185 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -56,3 +56,5 @@ hugeshmget02 hugeshmget02 -i 10
>  hugeshmget03 hugeshmget03 -i 10
>  hugeshmget05 hugeshmget05 -i 10
>  hugeshmget06 hugeshmget06 -i 10
> +
> +hugeshmmisc01 hugeshmmisc01
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index d88484fa1..8890d422e 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -48,6 +48,7 @@
>  /hugetlb/hugeshmget/hugeshmget03
>  /hugetlb/hugeshmget/hugeshmget05
>  /hugetlb/hugeshmget/hugeshmget06
> +/hugetlb/hugeshmmisc/hugeshmmisc01
>  /ksm/ksm01
>  /ksm/ksm02
>  /ksm/ksm03
> diff --git a/testcases/kernel/mem/hugetlb/hugeshmmisc/Makefile b/testcases/kernel/mem/hugetlb/hugeshmmisc/Makefile
> new file mode 100644
> index 000000000..84715c7b5
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugeshmmisc/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2009, Cisco Systems Inc.
> +# Ngie Cooper, July 2009
> +
> +top_srcdir              ?= ../../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(abs_srcdir)/../Makefile.inc
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/mem/hugetlb/hugeshmmisc/hugeshmmisc01.c b/testcases/kernel/mem/hugetlb/hugeshmmisc/hugeshmmisc01.c
> new file mode 100644
> index 000000000..7281401e8
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugeshmmisc/hugeshmmisc01.c
> @@ -0,0 +1,58 @@
> +#include "hugetlb.h"
> +#include "tst_safe_sysv_ipc.h"
> +
> +#define MNTPOINT "hugetlbfs/"
> +#define NR_HUGEPAGES 2
> +
> +static int shmid = -1;
> +static size_t size;
> +static size_t i;
> +static size_t ret;
> +
> +static volatile char *shmaddr;

The volatile shouldn't be here.

> +static int raw_fd;
> +static long hpage_size;
> +
> +static void setup(void)
> +{
> +	hpage_size = tst_get_hugepage_size();
> +}
> +
> +static void cleanup(void)
> +{
> +	if (shmid >= 0)
> +		SAFE_SHMCTL(shmid, IPC_RMID, NULL);
> +}
> +
> +static void run_test(void)
> +{
> +	size = hpage_size * NR_HUGEPAGES;
> +	raw_fd = SAFE_OPEN("/dev/random", O_RDONLY);
> +	tst_res(TINFO, "Requesting %zu bytes\n", size);

This message does not add any good information, I would remove it.

> +        shmid = SAFE_SHMGET(IPC_PRIVATE, size, SHM_HUGETLB|SHM_R|SHM_W);

We are not using the hugetlbfs/ since we get the hugepages via
SHM_HUGETLB, so we can remove the .mntpoint and .needs_hugetlbfs from
the tst_test structure.

> +	shmaddr = SAFE_SHMAT(shmid, 0, SHM_RND);
> +	tst_res(TINFO, "shmaddr: %p\n", shmaddr);
> +
> +	/* Read a page from device and write to shm segment */
> +	for (i = 0; i < size; i += hpage_size) {
> +		if (!read(raw_fd, shmaddr, hpage_size)) {
> +			tst_res(TFAIL | TERRNO, "Can't read from raw device!");
> +		}

TST_EXP_VAL(read(), hpage_size);

Also we may want to switch to "/dev/zero" instead of "/dev/random" as
"/dev/zero" should be faster.

> +	}
> +
> +	SAFE_SHMDT((const void *)shmaddr);

The cast to const void * shouldn't be there.

> +	tst_res(TPASS, "Test Passed!");

This is not going to be needed with TST_EXP_VAL();


> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.mntpoint = MNTPOINT,
> +	.needs_hugetlbfs = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run_test,
> +	.hugepages = {2, TST_NEEDS},
                      ^
		      This should be NR_HUGEPAGES
> +};

And lastly but not least, please use 'make check' and fix all the
problems reported, before sending a patch.

-- 
Cyril Hrubis
chrubis@suse.cz