[PATCH 31/38] Fix dt_pebs_init() call

[eugene.loh@oracle.com]

From: Eugene Loh <eugene.loh@oracle.com>

The function had a few issues, mainly with its comments,
ranging from typos to incorrect descriptions of behavior.

The function has only one caller.  The enforcement of a
size minimum was problematic in a number of respects:
- it was missing 4 bytes
- it was enforcing the minimum before the size was
    increased by the caller
- it was checking dt_pebs_init()==-1,
    missing errors with other negative return values

So change the function to accept a minimum and change its
return values.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_peb.c                          | 27 ++++++++++-----------
 libdtrace/dt_peb.h                          |  2 +-
 libdtrace/dt_work.c                         | 15 +++++++-----
 test/unittest/options/err.b-too-low.d       | 22 +++++++++--------
 test/unittest/options/err.bufsize-too-low.d | 22 +++++++++--------
 test/unittest/options/tst.b.d               | 22 +++++++++--------
 test/unittest/options/tst.bufsize.d         | 22 +++++++++--------
 7 files changed, 71 insertions(+), 61 deletions(-)

diff --git a/libdtrace/dt_peb.c b/libdtrace/dt_peb.c
index 5268f089..4983ba91 100644
--- a/libdtrace/dt_peb.c
+++ b/libdtrace/dt_peb.c
@@ -136,18 +136,14 @@ dt_pebs_exit(dtrace_hdl_t *dtp)
 }
 
 /*
- * Initialize the perf event buffers (one per online CPU).  Each buffer will
- * the given number of pages (i.e. the total size of each buffer will be
- * num_pages * getpagesize()).  The  allocated memory for each buffer is mmap'd
- * so the kernel can write to it, and its representative file descriptor is
- * recorded in the 'buffers' BPF map so that BPF code knows where to write
- * trace data for a specific CPU.
+ * Initialize the perf event buffers, one per online CPU.  Each buffer will
+ * have num_pages * getpagesize()).  The dt_peb_open() call mmaps the allocated
+ * memory so the kernel can write to it.  The file descriptor is recorded in
+ * the 'buffers' BPF map and added to the event polling file descriptor.
  *
- * An event polling file descriptor is created as well, and it is configured to
- * monitor all perf event buffers at once.  This file descriptor is returned
- * upon success..  Failure is indicated with a -1 return value.
+ * The return value indicates success (0) or failure (-1).
  */
-int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
+int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize, size_t bufsizemin)
 {
 	int		i;
 	int		mapfd;
@@ -166,12 +162,15 @@ int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
 		fprintf(stderr, "bufsize increased to %lu\n",
 			num_pages * getpagesize());
 
+	if (num_pages * getpagesize() < bufsizemin)
+		return dt_set_errno(dtp, EDT_BUFTOOSMALL);
+
 	/*
 	 * Determine the fd for the 'buffers' BPF map.
 	 */
 	idp = dt_dlib_get_map(dtp, "buffers");
 	if (idp == NULL || idp->di_id == DT_IDENT_UNDEF)
-		return -ENOENT;
+		return dt_set_errno(dtp, EDT_NOMEM);    // FIXME we used to do something more akin to ENOENT
 
 	mapfd = idp->di_id;
 
@@ -180,7 +179,7 @@ int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
 	 */
 	dtp->dt_pebset = dt_zalloc(dtp, sizeof(dt_pebset_t));
 	if (dtp->dt_pebset == NULL)
-		return -ENOMEM;
+		return dt_set_errno(dtp, EDT_NOMEM);
 
 	/*
 	 * Allocate the per-CPU perf event buffers.
@@ -189,7 +188,7 @@ int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
 			 sizeof(struct dt_peb));
 	if (pebs == NULL) {
 		dt_free(dtp, dtp->dt_pebset);
-		return -ENOMEM;
+		return dt_set_errno(dtp, EDT_NOMEM);
 	}
 
 	dtp->dt_pebset->pebs = pebs;
@@ -241,5 +240,5 @@ int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
 fail:
 	dt_pebs_exit(dtp);
 
-	return -1;
+	return dt_set_errno(dtp, EDT_NOMEM);  // FIXME need something better
 }
diff --git a/libdtrace/dt_peb.h b/libdtrace/dt_peb.h
index e0f408f2..9ad23252 100644
--- a/libdtrace/dt_peb.h
+++ b/libdtrace/dt_peb.h
@@ -43,7 +43,7 @@ typedef struct dt_pebset {
 } dt_pebset_t;
 
 extern void dt_pebs_exit(dtrace_hdl_t *);
-extern int dt_pebs_init(dtrace_hdl_t *, size_t);
+extern int dt_pebs_init(dtrace_hdl_t *, size_t, size_t);
 
 #ifdef	__cplusplus
 }
diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
index 69a86358..7a0eb1da 100644
--- a/libdtrace/dt_work.c
+++ b/libdtrace/dt_work.c
@@ -267,18 +267,21 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
 		return dt_set_errno(dtp, errno);
 
 	/*
-	 * We need enough space for the pref_event_header, a 32-bit size, a
+	 * We need enough space for the perf_event_header, a 32-bit size, a
 	 * 4-byte gap, and the largest trace data record we may be writing to
 	 * the buffer.  In other words, the buffer needs to be large enough to
 	 * hold at least one perf-encapsulated trace data record.
+	 *
+	 * While dt_pebs_init() rounds the requested size up, size==0 is a
+	 * special case.
 	 */
 	dtrace_getopt(dtp, "bufsize", &size);
-	if (size == 0 ||
-	    size < sizeof(struct perf_event_header) + sizeof(uint32_t) +
-		   dtp->dt_maxreclen)
+	if (size == 0)
 		return dt_set_errno(dtp, EDT_BUFTOOSMALL);
-	if (dt_pebs_init(dtp, size) == -1)
-		return dt_set_errno(dtp, EDT_NOMEM);
+	if (dt_pebs_init(dtp, size,
+			 sizeof(struct perf_event_header) + sizeof(uint32_t)
+			 + 4 + dtp->dt_maxreclen) == -1)
+		return -1;
 
 	/*
 	 * We must initialize the aggregation consumer handling before we
diff --git a/test/unittest/options/err.b-too-low.d b/test/unittest/options/err.b-too-low.d
index bb77e37c..f62155dd 100644
--- a/test/unittest/options/err.b-too-low.d
+++ b/test/unittest/options/err.b-too-low.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -12,19 +12,21 @@
  */
 
 /*
- * We use a buffer size of 59 because that should be just too small to hold the
- * trace records generated in this script:
- *	- perf_event_header (40 bytes)
- *	- size (4 bytes)
- *	- gap (4 bytes)
- *	- EPID (4 bytes)
- *	- tag (4 bytes)
- *	- exit value (4 bytes)
+ * We need over 4k bytes for the 4 string trace records generated in this script
+ * plus some meta data.
+ * A bufsize of 1 to 4096 bytes is rounded up to 4096 and is insufficient.
+ * A bufsize of 4097 to 8192 bytes is rounded up to 8192 and is sufficient.
  */
-/* @@runtest-opts: -b59 */
+/* @@runtest-opts: -b4096 */
+
+#pragma D option strsize=1024
 
 BEGIN
 {
+	trace("abc");
+	trace("def");
+	trace("ghi");
+	trace("jkl");
 	exit(0);
 }
 
diff --git a/test/unittest/options/err.bufsize-too-low.d b/test/unittest/options/err.bufsize-too-low.d
index bbbdb5c5..25efdf72 100644
--- a/test/unittest/options/err.bufsize-too-low.d
+++ b/test/unittest/options/err.bufsize-too-low.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -12,19 +12,21 @@
  */
 
 /*
- * We use a buffer size of 59 because that should be just too small to hold the
- * trace records generated in this script:
- *	- perf_event_header (40 bytes)
- *	- size (4 bytes)
- *	- gap (4 bytes)
- *	- EPID (4 bytes)
- *	- tag (4 bytes)
- *	- exit value (4 bytes)
+ * We need over 4k bytes for the 4 string trace records generated in this script
+ * plus some meta data.
+ * A bufsize of 1 to 4096 bytes is rounded up to 4096 and is insufficient.
+ * A bufsize of 4097 to 8192 bytes is rounded up to 8192 and is sufficient.
  */
-/* @@runtest-opts: -xbufsize=59 */
+/* @@runtest-opts: -xbufsize=4096 */
+
+#pragma D option strsize=1024
 
 BEGIN
 {
+	trace("abc");
+	trace("def");
+	trace("ghi");
+	trace("jkl");
 	exit(0);
 }
 
diff --git a/test/unittest/options/tst.b.d b/test/unittest/options/tst.b.d
index 57fa030d..3bf08edc 100644
--- a/test/unittest/options/tst.b.d
+++ b/test/unittest/options/tst.b.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -12,19 +12,21 @@
  */
 
 /*
- * We use a buffer size of 60 because that should be the exact size necessary
- * to hold the trace records generated in this script:
- *	- perf_event_header (40 bytes)
- *	- size (4 bytes)
- *	- gap (4 bytes)
- *	- EPID (4 bytes)
- *	- tag (4 bytes)
- *	- exit value (4 bytes)
+ * We need over 4k bytes for the 4 string trace records generated in this script
+ * plus some meta data.
+ * A bufsize of 1 to 4096 bytes is rounded up to 4096 and is insufficient.
+ * A bufsize of 4097 to 8192 bytes is rounded up to 8192 and is sufficient.
  */
-/* @@runtest-opts: -b60 */
+/* @@runtest-opts: -b4097 */
+
+#pragma D option strsize=1024
 
 BEGIN
 {
+	trace("abc");
+	trace("def");
+	trace("ghi");
+	trace("jkl");
 	exit(0);
 }
 
diff --git a/test/unittest/options/tst.bufsize.d b/test/unittest/options/tst.bufsize.d
index 96b0f1b8..23af81aa 100644
--- a/test/unittest/options/tst.bufsize.d
+++ b/test/unittest/options/tst.bufsize.d
@@ -1,6 +1,6 @@
 /*
  * Oracle Linux DTrace.
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -12,19 +12,21 @@
  */
 
 /*
- * We use a buffer size of 60 because that should be the exact size necessary
- * to hold the trace records generated in this script:
- *	- perf_event_header (40 bytes)
- *	- size (4 bytes)
- *	- gap (4 bytes)
- *	- EPID (4 bytes)
- *	- tag (4 bytes)
- *	- exit value (4 bytes)
+ * We need over 4k bytes for the 4 string trace records generated in this script
+ * plus some meta data.
+ * A bufsize of 1 to 4096 bytes is rounded up to 4096 and is insufficient.
+ * A bufsize of 4097 to 8192 bytes is rounded up to 8192 and is sufficient.
  */
-/* @@runtest-opts: -xbufsize=60 */
+/* @@runtest-opts: -xbufsize=4097 */
+
+#pragma D option strsize=1024
 
 BEGIN
 {
+	trace("abc");
+	trace("def");
+	trace("ghi");
+	trace("jkl");
 	exit(0);
 }
 
-- 
2.18.4