[LTP] [PATCH] [PATCH V6] syscalls: ipc: Add shmget hugepage test

[LTP] [PATCH] [PATCH V6] syscalls: ipc: Add shmget hugepage test

[Pavithra]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3248 bytes --]

Changes in v6:
- Added "tst_test.h" and "tst_hugepage.h" instead of hugetlb.h
- Modified code to use TEST() macro

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
 runtest/syscalls                              |  1 +
 .../kernel/syscalls/ipc/shmget/.gitignore     |  1 +
 .../kernel/syscalls/ipc/shmget/shmget07.c     | 75 +++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ipc/shmget/shmget07.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 2179e007c..1d5e0f214 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1547,6 +1547,7 @@ shmget03 shmget03
 shmget04 shmget04
 shmget05 shmget05
 shmget06 shmget06
+shmget07 shmget07
 
 shutdown01 shutdown01
 shutdown02 shutdown02
diff --git a/testcases/kernel/syscalls/ipc/shmget/.gitignore b/testcases/kernel/syscalls/ipc/shmget/.gitignore
index 768d1c69d..23ba002b5 100644
--- a/testcases/kernel/syscalls/ipc/shmget/.gitignore
+++ b/testcases/kernel/syscalls/ipc/shmget/.gitignore
@@ -3,3 +3,4 @@
 /shmget04
 /shmget05
 /shmget06
+/shmget07
diff --git a/testcases/kernel/syscalls/ipc/shmget/shmget07.c b/testcases/kernel/syscalls/ipc/shmget/shmget07.c
new file mode 100644
index 000000000..c6e4068d8
--- /dev/null
+++ b/testcases/kernel/syscalls/ipc/shmget/shmget07.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
+ */
+
+/*\
+ * [Descripiton]
+ *
+ * Origin: https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/shm-getraw.c
+ *
+ * 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
+ *
+ */
+
+#include "tst_test.h"
+#include "tst_hugepage.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 char *shmaddr;
+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/zero", O_RDONLY);
+
+	shmid = SAFE_SHMGET(IPC_PRIVATE, size, SHM_HUGETLB|SHM_R|SHM_W);
+
+	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) {
+		TEST(read(raw_fd, shmaddr, hpage_size));
+		if (TST_RET != hpage_size)
+			tst_res(TFAIL | TERRNO, "Can't read full page from raw device!");
+		else
+			tst_res(TPASS, "Read full page from raw device");
+	}
+
+	SAFE_SHMDT(shmaddr);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+	.hugepages = {2, TST_NEEDS},
+};
-- 
2.53.0


[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] [PATCH V6] syscalls: ipc: Add shmget hugepage test

[Andrea Cervesato via ltp]

Hi Pavithra,

A few issues to address before this can be merged.

> Changes in v6:
> - Added "tst_test.h" and "tst_hugepage.h" instead of hugetlb.h
> - Modified code to use TEST() macro

The commit body should explain *why* the test is being added — what
behavior it validates or what gap it fills. Changelog entries belong
below the --- separator in the patch email, not in the commit message
body. Please write a proper description here.

> + * [Descripiton]

Two problems: "Descripiton" is a typo, and the [Description] label
itself is deprecated in LTP doc comments. Drop the label entirely and
write the description text directly.

> +static int raw_fd;

raw_fd is zero-initialized by the compiler but fd 0 is stdin. More
importantly, raw_fd is opened in run_test() and never closed anywhere —
not at the end of run_test(), not in cleanup(). This leaks a file
descriptor per test invocation.

Initialize it to -1, close it at the end of run_test(), and guard it
in cleanup():

  static int raw_fd = -1;

  /* in cleanup(): */
  if (raw_fd != -1)
      SAFE_CLOSE(raw_fd);

> +static size_t size;
> +static size_t i;

These are only used inside run_test() and do not carry state between
calls. `i` should be declared as local and `size` can be initialized
inside the setup() function.

> +	shmid = SAFE_SHMGET(IPC_PRIVATE, size, SHM_HUGETLB|SHM_R|SHM_W);
> +
> +	shmaddr = SAFE_SHMAT(shmid, 0, SHM_RND);
> +	tst_res(TINFO, "shmaddr: %p\n", shmaddr);

Two issues here:

1. tst_res() appends its own newline — the "\n" in the format string
   produces a spurious blank line in the output. Remove it.

2. shmaddr is not tracked in cleanup(). If run_test() is aborted after
   SAFE_SHMAT (e.g. by a tst_brk() from a future change or a signal),
   the segment remains attached. Add to cleanup():

     if (shmaddr)
         SAFE_SHMDT(shmaddr);

   And reset shmaddr = NULL after SAFE_SHMDT in run_test().

> +	SAFE_SHMDT(shmaddr);
> +}

When the test is run with -i N, each call to run_test() overwrites
shmid and raw_fd with fresh values without releasing the previous
iteration's resources:

  - The previous raw_fd is never closed   -> fd leak per iteration.
  - The previous shmid is never IPC_RMID'd before being overwritten
    -> shm segment leak per iteration; cleanup() only removes the last.

Remove shmid at the end of run_test() and reset it to -1 there, so
cleanup() is only a safety net. Close raw_fd at the end of run_test()
as well.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp