Linux Test Project
 help / color / mirror / Atom feed
* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-07-17 11:02 [LTP] [PATCH] " Pavithra
@ 2026-07-17 11:28 ` linuxtestproject.agent
  0 siblings, 0 replies; 9+ messages in thread
From: linuxtestproject.agent @ 2026-07-17 11:28 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Fri, 17 Jul 2026 16:32:34 +0530, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> This test verifies that ptrace POKEDATA and PEEKDATA operations work
> correctly on hugepage-backed memory regions.

The commit body describes what the test does but does not say why the
migration from libhugetlbfs is happening. What is the motivation?
(e.g. libhugetlbfs is deprecated and tests are being consolidated into
LTP.) The rule requires the body to state the motivation or the problem
being solved.

> +	err = ptrace(PTRACE_ATTACH, cpid, NULL, NULL);
> +	if (err)
> +		tst_brk(TFAIL | TERRNO, "ptrace(ATTACH) failed");

PTRACE_ATTACH is a setup step, not the operation under test. A failure
here means the test could not be set up, which should be TBROK, not
TFAIL. TFAIL implies the hugepage ptrace feature is broken, which is
misleading when the real cause is, for example, a restrictive
ptrace_scope.

The doc block already notes: "for ptrace(PTRACE_ATTACH) to work across
processes when ptrace_scope is restrictive." At ptrace_scope=3, even
root gets EPERM, and the correct response is TCONF, not TFAIL.

Consider:

  SAFE_PTRACE(PTRACE_ATTACH, cpid, NULL, NULL);

or, to handle the ptrace_scope case explicitly:

  if (ptrace(PTRACE_ATTACH, cpid, NULL, NULL)) {
      if (errno == EPERM)
          tst_brk(TCONF | TERRNO, "ptrace(ATTACH) not permitted");
      tst_brk(TBROK | TERRNO, "ptrace(ATTACH) failed");
  }

> +	err = ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST);
> +	if (err)
> +		tst_brk(TFAIL | TERRNO, "ptrace(POKEDATA) failed");
> +
> +	tst_res(TINFO, "Peeking at %p...", p);
> +	errno = 0;
> +	err = ptrace(PTRACE_PEEKDATA, pid, p, NULL);
> +	if (err == -1 && errno)
> +		tst_brk(TFAIL | TERRNO, "ptrace(PEEKDATA) failed");

PTRACE_POKEDATA and PTRACE_PEEKDATA are the syscalls under test and
must not be called bare. The rule requires subject syscalls to be
wrapped in TEST() or TST_EXP_*. SAFE_PTRACE cannot be used for
PEEKDATA (it treats any non-zero return as error, but PEEKDATA returns
arbitrary data), so TEST() is the right wrapper for both. Example:

  TEST(ptrace(PTRACE_POKEDATA, pid, p, (void *)CONST));
  if (TST_RET != 0)
      tst_brk(TFAIL | TTERRNO, "ptrace(POKEDATA) failed");

  errno = 0;
  TEST(ptrace(PTRACE_PEEKDATA, pid, p, NULL));
  if (TST_RET == -1 && TST_ERR)
      tst_brk(TFAIL | TTERRNO, "ptrace(PEEKDATA) failed");

> +	pause();
> +	/* Child is killed by parent via PTRACE_KILL, so cleanup is not reached */

The comment says PTRACE_KILL, but the parent uses SAFE_KILL(cpid,
SIGKILL). PTRACE_KILL is a deprecated ptrace request, not what is
used here. The comment is factually wrong.

> + * This test verifies that ptrace POKEDATA and PEEKDATA work correctly
> + * on hugepage-backed memory regions. A child process maps a hugepage,
> + * and the parent uses ptrace to write and read data from the child's
> + * hugepage memory, ensuring that ptrace operations function properly
> + * with hugepage mappings.

The doc block references the ptrace syscall without the required
manpage RST role. Use :manpage:`ptrace(2)` when referring to a syscall
in a /*\ ... */ block.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-07-17 18:05 [LTP] [PATCH] " Pavithra
@ 2026-07-17 19:13 ` linuxtestproject.agent
  2026-08-04 12:25   ` Cyril Hrubis
  0 siblings, 1 reply; 9+ messages in thread
From: linuxtestproject.agent @ 2026-07-17 19:13 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Fri, 17 Jul 2026 23:35:33 +0530, Pavithra <pavrampu@linux.ibm.com> wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

Verdict - Reviewed

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-07-17 19:13 ` [LTP] " linuxtestproject.agent
@ 2026-08-04 12:25   ` Cyril Hrubis
  2026-08-04 12:28     ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2026-08-04 12:25 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: Pavithra, ltp

Hi!
@Andrea looks like the CI missed a few things in the code, see my
review.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-08-04 12:25   ` Cyril Hrubis
@ 2026-08-04 12:28     ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 9+ messages in thread
From: Andrea Cervesato via ltp @ 2026-08-04 12:28 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: Pavithra, ltp, linuxtestproject.agent

> @Andrea looks like the CI missed a few things in the code, see my
> review.

I think at that point we were still using Sonnet which is not the
sweet spot. Now it should be better.

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [LTP] [PATCH v5] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
@ 2026-08-08 15:02 Pavithra
  2026-08-08 15:30 ` [LTP] " linuxtestproject.agent
  0 siblings, 1 reply; 9+ messages in thread
From: Pavithra @ 2026-08-08 15:02 UTC (permalink / raw)
  To: ltp; +Cc: pavrampu

This test verifies that ptrace POKEDATA and PEEKDATA operations work
correctly on hugepage-backed memory regions. Migrating it to LTP as
libhugetlbfs is not actively maintained.

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
v4 -> v5:
- Replace ptrace PTRACE_POKEDATA and PTRACE_ATTACH calls with SAFE_PTRACE().
- Use TST_EXP_EQ_LI() to verify peeked memory values.
- Replace TST_PROCESS_STATE_WAIT() with SAFE_WAITPID() and WIFSTOPPED() check.
- Simplify SAFE_CLOSE() cleanup logic and remove redundant comments.
- Reduce required hugepages from 4 to 1.
link to v1: https://lore.kernel.org/ltp/20260717180533.1203314-1-pavrampu@linux.ibm.com/
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap41.c  | 136 ++++++++++++++++++
 3 files changed, 138 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 6b35c1f42..621c9718a 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -40,6 +40,7 @@ hugemmap35 hugemmap35
 hugemmap36 hugemmap36
 hugemmap37 hugemmap37
 hugemmap38 hugemmap38
+hugemmap41 hugemmap41
 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 e63a6dde7..9e706e1c8 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -40,6 +40,7 @@
 /hugetlb/hugemmap/hugemmap36
 /hugetlb/hugemmap/hugemmap37
 /hugetlb/hugemmap/hugemmap38
+/hugetlb/hugemmap/hugemmap41
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
new file mode 100644
index 000000000..224de254f
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap41.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
+ * Copyright (c) 2026 Pavithra <pavrampu@linux.ibm.com>
+ */
+
+/*\
+ * Test :manpage:`ptrace(2)` write to hugepage memory.
+ *
+ * A child process maps a hugepage via hugetlbfs, zeroes it, and sends
+ * the mapped address to the parent. The parent attaches with
+ * :manpage:`ptrace(2)`, then uses PTRACE_POKEDATA and PTRACE_PEEKDATA
+ * to write and read back a known value at two different offsets within
+ * the hugepage, verifying that ptrace operates correctly on
+ * hugepage-backed memory regions.
+ *
+ * Requires root to mount hugetlbfs and for :manpage:`ptrace(2)`
+ * PTRACE_ATTACH to work across processes when ptrace_scope is
+ * restrictive.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "hugetlb.h"
+
+#define CONST	0xdeadbeefL
+#define MNTPOINT "hugetlbfs/"
+
+static long hpage_size;
+static int fd = -1;
+
+static void child(int hugefd, int pipefd[2])
+{
+	void *p;
+
+	SAFE_CLOSE(pipefd[0]);
+
+	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,
+		      hugefd, 0);
+
+	memset(p, 0, hpage_size);
+
+	tst_res(TINFO, "Child mapped data at %p", p);
+
+	SAFE_WRITE(SAFE_WRITE_ALL, pipefd[1], &p, sizeof(p));
+	SAFE_CLOSE(pipefd[1]);
+
+	pause();
+	/* Child is killed by parent via SIGKILL, so cleanup is not reached */
+}
+
+static void do_poke(pid_t pid, void *p)
+{
+	tst_res(TINFO, "Poking at %p...", p);
+	SAFE_PTRACE(PTRACE_POKEDATA, pid, p, (void *)CONST);
+
+	tst_res(TINFO, "Peeking at %p...", p);
+	TEST(ptrace(PTRACE_PEEKDATA, pid, p, NULL));
+	if (TST_ERR)
+		tst_brk(TFAIL | TTERRNO, "ptrace(PEEKDATA) failed");
+
+	TST_EXP_EQ_LI(TST_RET, CONST);
+}
+
+static void run_test(void)
+{
+	int pipefd[2];
+	pid_t cpid;
+	void *p;
+	int status;
+
+	fd = tst_creat_unlinked(MNTPOINT, 0, 0600);
+
+	SAFE_PIPE(pipefd);
+
+	cpid = SAFE_FORK();
+
+	if (cpid == 0) {
+		child(fd, pipefd);
+		exit(0);
+	}
+
+	SAFE_CLOSE(pipefd[1]);
+	SAFE_READ(1, pipefd[0], &p, sizeof(p));
+	SAFE_CLOSE(pipefd[0]);
+
+	tst_res(TINFO, "Parent received address %p", p);
+
+	SAFE_PTRACE(PTRACE_ATTACH, cpid, NULL, NULL);
+
+	SAFE_WAITPID(cpid, &status, 0);
+	if (!WIFSTOPPED(status))
+		tst_brk(TBROK, "Child %d was not stopped", cpid);
+
+	do_poke(cpid, p);
+	do_poke(cpid, p + getpagesize());
+
+	SAFE_KILL(cpid, SIGKILL);
+	SAFE_WAITPID(cpid, &status, 0);
+
+	SAFE_CLOSE(fd);
+
+	tst_res(TPASS, "ptrace write to hugepage succeeded");
+}
+
+static void setup(void)
+{
+	hpage_size = tst_get_hugepage_size();
+}
+
+static void cleanup(void)
+{
+	if (fd != -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.tags = (struct tst_tag[]) {
+		{"linux-git", "ebed4bfc8da8"},
+		{}
+	},
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.hugepages = {1, TST_NEEDS},
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+};
-- 
2.55.0


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-08-08 15:02 [LTP] [PATCH v5] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs Pavithra
@ 2026-08-08 15:30 ` linuxtestproject.agent
  0 siblings, 0 replies; 9+ messages in thread
From: linuxtestproject.agent @ 2026-08-08 15:30 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Sat, 08 Aug 2026, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> +#define CONST	0xdeadbeefL
> [...]
> +	TST_EXP_EQ_LI(TST_RET, CONST);

Could this comparison preserve unsigned-long semantics? On 32-bit targets,
CONST is an unsigned long, while PTRACE_PEEKDATA returns the same bit pattern
as a negative signed long. TST_EXP_EQ_LI() converts them separately to signed
long long, producing -559038737 and 3735928559, so every successful peek is
reported as TFAIL.

> +/hugetlb/hugemmap/hugemmap41

Could this entry be placed in a .gitignore under
testcases/kernel/mem/hugetlb/hugemmap/? New test binaries should be ignored
in their own test directory rather than in a parent .gitignore.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-08-08 17:02 [LTP] [PATCH v6] " Pavithra
@ 2026-08-08 17:33 ` linuxtestproject.agent
  0 siblings, 0 replies; 9+ messages in thread
From: linuxtestproject.agent @ 2026-08-08 17:33 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Sat, Aug 8, 2026, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> +/hugetlb/hugemmap/hugemmap41

Could this entry be placed in
testcases/kernel/mem/hugetlb/hugemmap/.gitignore as /hugemmap41 instead?
New test binaries should be ignored in their leaf test directory rather
than extending the legacy testcases/kernel/mem/.gitignore list.

> +	p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED,

Could spaces be added around the "|" operator? make check-hugemmap41
reports this line as a kernel style violation.

> +	TST_EXP_EQ_LU((unsigned long)TST_RET, CONST);
> +}
> +
> +static void run_test(void)
> +{
> [...]
> +	do_poke(cpid, p);
> +	do_poke(cpid, p + getpagesize());
> [...]
> +	tst_res(TPASS, "ptrace write to hugepage succeeded");

Could the unconditional TPASS be removed? If either equality check reports
a mismatch, this line still claims that the operation succeeded. The two
TST_EXP_EQ_LU() calls already report their own pass results.

> +	.tags = (struct tst_tag[]) {
> +		{"linux-git", "ebed4bfc8da8"},
> +		{}
> +	},

Which ptrace regression is this tag intended to identify? ebed4bfc8da8 is
"hugetlb: fix absurd HugePages_Rsvd", which fixes reservation accounting
after truncating a mapped hugetlbfs file. This test neither truncates the
file nor checks HugePages_Rsvd, so a failure would print an unrelated
missing-fix hint.

Could the tag be removed, or replaced with the full 40-character hash of
the actual ptrace/hugetlb fix?

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-08-09 17:16 [LTP] [PATCH] " Pavithra
@ 2026-08-09 17:33 ` linuxtestproject.agent
  0 siblings, 0 replies; 9+ messages in thread
From: linuxtestproject.agent @ 2026-08-09 17:33 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Sun, 9 Aug 2026, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> +	tst_res(TINFO, "Poking at %p...", p);
> +	SAFE_PTRACE(PTRACE_POKEDATA, pid, p, (void *)CONST);

Could this call use TEST() or a suitable TST_EXP_* check and report failure
as TFAIL? PTRACE_POKEDATA is an operation under test, but SAFE_PTRACE()
converts a kernel regression here into TBROK.

> +/hugetlb/hugemmap/hugemmap41

Could this entry instead be added as /hugemmap41 in
testcases/kernel/mem/hugetlb/hugemmap/.gitignore? LTP keeps generated test
binary entries in the leaf test directory so they move or disappear with
that directory.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs
  2026-08-09 18:50 [LTP] [PATCH v8] " Pavithra
@ 2026-08-09 19:37 ` linuxtestproject.agent
  0 siblings, 0 replies; 9+ messages in thread
From: linuxtestproject.agent @ 2026-08-09 19:37 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

On Mon, 10 Aug 2026, Pavithra wrote:
> hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs

> +	SAFE_PTRACE(PTRACE_ATTACH, cpid, NULL, NULL);

Could PTRACE_ATTACH be handled without SAFE_PTRACE() so that EPERM caused
by Yama policy can be reported as TCONF? With ptrace_scope=3 Linux denies
every attach, including requests from root, and the wrapper converts this
expected policy denial to TBROK before the test reaches PTRACE_POKEDATA.

> +/hugetlb/hugemmap/hugemmap41

Could this entry instead be placed in a .gitignore in the hugemmap leaf
directory? LTP requires new test binaries to be ignored in their own test
directory rather than from a parent directory.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-08-09 19:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 15:02 [LTP] [PATCH v5] hugemmap/hugemmap41: Migrate ptrace-write-hugepage from libhugetlbfs Pavithra
2026-08-08 15:30 ` [LTP] " linuxtestproject.agent
  -- strict thread matches above, loose matches on Subject: below --
2026-08-09 18:50 [LTP] [PATCH v8] " Pavithra
2026-08-09 19:37 ` [LTP] " linuxtestproject.agent
2026-08-09 17:16 [LTP] [PATCH] " Pavithra
2026-08-09 17:33 ` [LTP] " linuxtestproject.agent
2026-08-08 17:02 [LTP] [PATCH v6] " Pavithra
2026-08-08 17:33 ` [LTP] " linuxtestproject.agent
2026-07-17 18:05 [LTP] [PATCH] " Pavithra
2026-07-17 19:13 ` [LTP] " linuxtestproject.agent
2026-08-04 12:25   ` Cyril Hrubis
2026-08-04 12:28     ` Andrea Cervesato via ltp
2026-07-17 11:02 [LTP] [PATCH] " Pavithra
2026-07-17 11:28 ` [LTP] " linuxtestproject.agent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox