[DTrace-devel] [PATCH] bufsize considered too small even when increased sufficiently

eugene.loh at oracle.com eugene.loh at oracle.com
Fri Oct 23 16:47:53 PDT 2020


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

The buffer size can be judged too small, even though it is increased
more than sufficiently.  E.g., consider

    # dtrace -x bufsize=32 -qn 'BEGIN { trace(1); trace(2); exit(0) }'
    bufsize increased to 4096
    12
    # dtrace -x bufsize=32 -qn 'BEGIN { trace(1); trace(2);
                                                  trace(3); exit(0) }'
    dtrace: could not enable tracing: Enabling exceeds size of buffer

The first case shows we rounded the buffer size up to a full page.
The second case shows a small-buffer error even though 4k is plenty.

Change dtrace_go() so that it does not check bufsize before calling
dt_pebs_init(), but simply pass a minimum size in to be checked.
Add a test for this problem.

Orabug: 32065592
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_peb.c                            | 34 ++++++++++++-------
 libdtrace/dt_peb.h                            |  2 +-
 libdtrace/dt_work.c                           | 15 ++++----
 test/unittest/buffering/tst.increasebufsize.d | 32 +++++++++++++++++
 test/unittest/buffering/tst.increasebufsize.r |  6 ++++
 5 files changed, 67 insertions(+), 22 deletions(-)
 create mode 100644 test/unittest/buffering/tst.increasebufsize.d
 create mode 100644 test/unittest/buffering/tst.increasebufsize.r

diff --git a/libdtrace/dt_peb.c b/libdtrace/dt_peb.c
index 0ef1c419..6355605c 100644
--- a/libdtrace/dt_peb.c
+++ b/libdtrace/dt_peb.c
@@ -146,31 +146,35 @@ dt_pebs_exit(dtrace_hdl_t *dtp)
  * monitor all perf event buffers at once.  This file descriptor is returned
  * upon success..  Failure is indicated with a -1 return value.
  */
-int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
+int dt_pebs_init(dtrace_hdl_t *dtp, size_t reqsize, size_t minsize)
 {
 	int		i;
 	int		mapfd;
-	size_t		num_pages;
+	size_t		bufsize;
 	dt_ident_t	*idp;
 	dt_peb_t	*pebs;
 
 	/*
 	 * The perf event buffer implementation in the kernel requires that the
 	 * buffer comprises n full memory pages, where n must be a power of 2.
-	 * Since the buffer size is specified in bytes in DTrace, we need to
-	 * convert this size (and possibly round it up) to an acceptable value.
+	 * Since the buffer size is specified in bytes in DTrace, we possibly
+	 * need to round it up to an acceptable value.
 	 */
-	num_pages = roundup_pow2((bufsize + getpagesize() - 1) / getpagesize());
-	if (num_pages * getpagesize() > bufsize)
-		fprintf(stderr, "bufsize increased to %lu\n",
-			num_pages * getpagesize());
+	bufsize = roundup_pow2((reqsize + getpagesize() - 1) / getpagesize());
+	bufsize *= getpagesize();
+	if (bufsize > reqsize)
+		fprintf(stderr, "bufsize increased to %lu\n", bufsize);
+	if (bufsize < minsize)
+		return dt_set_errno(dtp, EDT_BUFTOOSMALL);
 
 	/*
 	 * Determine the fd for the 'buffers' BPF map.
+	 * FIXME:  The errno is not right, but this was the existing
+	 *     behavior and no other EDT_* fits better.
 	 */
 	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);
 
 	mapfd = idp->di_id;
 
@@ -179,7 +183,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.
@@ -188,13 +192,13 @@ 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;
 
 	dtp->dt_pebset->page_size = getpagesize();
-	dtp->dt_pebset->data_size = num_pages * dtp->dt_pebset->page_size;
+	dtp->dt_pebset->data_size = bufsize;
 
 	/*
 	 * Initialize a perf event buffer for each online CPU.
@@ -240,5 +244,9 @@ int dt_pebs_init(dtrace_hdl_t *dtp, size_t bufsize)
 fail:
 	dt_pebs_exit(dtp);
 
-	return -1;
+	/*
+	 * FIXME:  The errno is not right, but this was the existing
+	 *     behavior and no other EDT_* fits better.
+	 */
+	return dt_set_errno(dtp, EDT_NOMEM);
 }
diff --git a/libdtrace/dt_peb.h b/libdtrace/dt_peb.h
index 961db93c..03ee7df7 100644
--- a/libdtrace/dt_peb.h
+++ b/libdtrace/dt_peb.h
@@ -42,7 +42,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 02246219..737b173a 100644
--- a/libdtrace/dt_work.c
+++ b/libdtrace/dt_work.c
@@ -143,7 +143,8 @@ dtrace_status(dtrace_hdl_t *dtp)
 int
 dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
 {
-	size_t		size;
+	size_t		reqsize;
+	size_t		minsize;
 	int		err;
 
 	if (dtp->dt_active)
@@ -186,13 +187,11 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
 	 * the buffer.  In other words, the buffer needs to be large enough to
 	 * hold at least one perf-encapsulated trace data record.
 	 */
-	dtrace_getopt(dtp, "bufsize", &size);
-	if (size == 0 ||
-	    size < sizeof(struct perf_event_header) + sizeof(uint32_t) +
-		   dtp->dt_maxreclen)
-		return dt_set_errno(dtp, EDT_BUFTOOSMALL);
-	if (dt_pebs_init(dtp, size) == -1)
-		return dt_set_errno(dtp, EDT_NOMEM);
+	dtrace_getopt(dtp, "bufsize", &reqsize);
+	minsize = sizeof(struct perf_event_header) + sizeof(uint32_t) +
+	   dtp->dt_maxreclen;
+	if (dt_pebs_init(dtp, reqsize, minsize) == -1)
+		return -1;
 
 	BEGIN_probe();
 
diff --git a/test/unittest/buffering/tst.increasebufsize.d b/test/unittest/buffering/tst.increasebufsize.d
new file mode 100644
index 00000000..e4a2fbd5
--- /dev/null
+++ b/test/unittest/buffering/tst.increasebufsize.d
@@ -0,0 +1,32 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2020, 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.
+ */
+
+/*
+ * ASSERTION:
+ *   A small buffer size will be increased automatically.
+ *
+ * SECTION: Buffers.
+ *	Buffers and Buffering/Buffer Sizes;
+ *	Options and Tunables/bufsize;
+ */
+
+#pragma D option bufsize=16
+
+BEGIN
+{
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	trace(12345); trace(67890);
+	exit(0);
+}
diff --git a/test/unittest/buffering/tst.increasebufsize.r b/test/unittest/buffering/tst.increasebufsize.r
new file mode 100644
index 00000000..0616c681
--- /dev/null
+++ b/test/unittest/buffering/tst.increasebufsize.r
@@ -0,0 +1,6 @@
+                   FUNCTION:NAME
+                          :BEGIN       12345      67890      12345      67890      12345      67890      12345      67890      12345      67890      12345      67890      12345      67890      12345      67890      12345      67890      12345      67890
+
+-- @@stderr --
+dtrace: script 'test/unittest/buffering/tst.increasebufsize.d' matched 1 probe
+bufsize increased to 4096
-- 
2.18.4




More information about the DTrace-devel mailing list