debug: move all of the debug bits out of the spl

behlendorf · robn · commit 4c008827e283 · 2025-11-05T08:48:44.000+11:00
Pull all of the internal debug infrastructure up in to the zfs
code to clean up the layering.  Remove all the dodgy usage of
SET_ERROR and DTRACE_PROBE from the spl.  Luckily it was
lightly used in the spl layer so we're not losing much.

Signed-off-by: Brian Behlendorf &lt;behlendorf1@llnl.gov&gt;
Signed-off-by: Rob Norris &lt;robn@despairlabs.com&gt;
diff --git a/include/os/freebsd/Makefile.am b/include/os/freebsd/Makefile.am
@@ -44,7 +44,6 @@ noinst_HEADERS = \
 	%D%/spl/sys/procfs_list.h \
 	%D%/spl/sys/random.h \
 	%D%/spl/sys/rwlock.h \
-	%D%/spl/sys/sdt.h \
 	%D%/spl/sys/sid.h \
 	%D%/spl/sys/sig.h \
 	%D%/spl/sys/simd.h \
@@ -63,7 +62,6 @@ noinst_HEADERS = \
 	%D%/spl/sys/time.h \
 	%D%/spl/sys/timer.h \
 	%D%/spl/sys/trace.h \
-	%D%/spl/sys/trace_zfs.h \
 	%D%/spl/sys/types.h \
 	%D%/spl/sys/types32.h \
 	%D%/spl/sys/uio.h \
@@ -82,10 +80,12 @@ noinst_HEADERS = \
 	%D%/zfs/sys/arc_os.h \
 	%D%/zfs/sys/freebsd_crypto.h \
 	%D%/zfs/sys/freebsd_event.h \
+	%D%/zfs/sys/trace_zfs.h \
 	%D%/zfs/sys/vdev_os.h \
 	%D%/zfs/sys/zfs_bootenv_os.h \
 	%D%/zfs/sys/zfs_context_os.h \
 	%D%/zfs/sys/zfs_ctldir.h \
+	%D%/zfs/sys/zfs_debug_os.h \
 	%D%/zfs/sys/zfs_dir.h \
 	%D%/zfs/sys/zfs_ioctl_compat.h \
 	%D%/zfs/sys/zfs_vfsops_os.h \
diff --git a/include/os/freebsd/zfs/sys/trace_zfs.h b/include/os/freebsd/zfs/sys/trace_zfs.h
diff --git a/include/os/freebsd/zfs/sys/zfs_debug_os.h b/include/os/freebsd/zfs/sys/zfs_debug_os.h
@@ -27,10 +27,11 @@
  * $FreeBSD$
  */
 
-#ifndef _OPENSOLARIS_SYS_SDT_H_
-#define	_OPENSOLARIS_SYS_SDT_H_
+#ifndef _SYS_ZFS_DEBUG_OS_H
+#define	_SYS_ZFS_DEBUG_OS_H
+
+#include <sys/sdt.h>
 
-#include_next <sys/sdt.h>
 #ifdef KDTRACE_HOOKS
 SDT_PROBE_DECLARE(sdt, , , set__error);
 
@@ -44,4 +45,4 @@ SDT_PROBE_DECLARE(sdt, , , set__error);
 #define	SET_ERROR(err) (err)
 #endif
 
-#endif	/* _OPENSOLARIS_SYS_SDT_H_ */
+#endif	/* _SYS_ZFS_DEBUG_OS_H */
diff --git a/include/os/linux/Makefile.am b/include/os/linux/Makefile.am
@@ -41,6 +41,7 @@ kernel_sys_HEADERS = \
 	%D%/zfs/sys/zfs_bootenv_os.h \
 	%D%/zfs/sys/zfs_context_os.h \
 	%D%/zfs/sys/zfs_ctldir.h \
+	%D%/zfs/sys/zfs_debug_os.h \
 	%D%/zfs/sys/zfs_dir.h \
 	%D%/zfs/sys/zfs_vfsops_os.h \
 	%D%/zfs/sys/zfs_vnops_os.h \
@@ -97,7 +98,6 @@ kernel_spl_sys_HEADERS = \
 	%D%/spl/sys/time.h \
 	%D%/spl/sys/timer.h \
 	%D%/spl/sys/trace.h \
-	%D%/spl/sys/trace_spl.h \
 	%D%/spl/sys/trace_taskq.h \
 	%D%/spl/sys/tsd.h \
 	%D%/spl/sys/types.h \
diff --git a/include/os/linux/spl/sys/sysmacros.h b/include/os/linux/spl/sys/sysmacros.h
@@ -34,11 +34,6 @@
 #include <sys/signal.h>
 #include <asm/page.h>
 
-
-#ifndef _KERNEL
-#define	_KERNEL				__KERNEL__
-#endif
-
 #define	FALSE				0
 #define	TRUE				1
 
@@ -202,9 +197,6 @@ makedev(unsigned int major, unsigned int minor)
 #define	P2SAMEHIGHBIT_TYPED(x, y, type) \
 	(((type)(x) ^ (type)(y)) < ((type)(x) & (type)(y)))
 
-#define	SET_ERROR(err) \
-	(__set_error(__FILE__, __func__, __LINE__, err), err)
-
 #include <linux/sort.h>
 #define	qsort(base, num, size, cmp)		\
 	sort(base, num, size, cmp, NULL)
diff --git a/include/os/linux/zfs/sys/zfs_debug_os.h b/include/os/linux/zfs/sys/zfs_debug_os.h
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: CDDL-1.0
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or https://opensource.org/licenses/CDDL-1.0.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+#ifndef _SYS_ZFS_DEBUG_OS_H
+#define	_SYS_ZFS_DEBUG_OS_H
+
+#define	 SET_ERROR(err) \
+	(__set_error(__FILE__, __func__, __LINE__, err), err)
+
+#endif	/* _SYS_ZFS_DEBUG_OS_H */
diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h
@@ -109,7 +109,6 @@ extern "C" {
 #include <sys/sysevent/eventdefs.h>
 #include <sys/sunddi.h>
 #include <sys/debug.h>
-#include <sys/trace_zfs.h>
 #include <sys/zone.h>
 
 #include <sys/mutex.h>
diff --git a/include/sys/zfs_debug.h b/include/sys/zfs_debug.h
@@ -40,6 +40,7 @@ extern "C" {
 #endif
 
 #include <sys/nvpair.h>
+#include <sys/zfs_debug_os.h>
 
 extern int zfs_flags;
 extern int zfs_recover;
diff --git a/lib/libspl/include/Makefile.am b/lib/libspl/include/Makefile.am
@@ -69,7 +69,6 @@ libspl_sys_HEADERS = \
 	%D%/sys/time.h \
 	%D%/sys/timer.h \
 	%D%/sys/trace.h \
-	%D%/sys/trace_spl.h \
 	%D%/sys/tsd.h \
 	%D%/sys/tunables.h \
 	%D%/sys/types.h \
diff --git a/lib/libspl/include/sys/trace_spl.h b/lib/libspl/include/sys/trace_spl.h
diff --git a/lib/libzpool/include/Makefile.am b/lib/libzpool/include/Makefile.am
@@ -2,4 +2,5 @@ libzpool_sys_HEADERS = \
 	%D%/sys/abd_os.h \
 	%D%/sys/abd_impl_os.h \
 	%D%/sys/trace_zfs.h \
-	%D%/sys/zfs_context_os.h
+	%D%/sys/zfs_context_os.h \
+	%D%/sys/zfs_debug_os.h
diff --git a/lib/libzpool/include/sys/trace_zfs.h b/lib/libzpool/include/sys/trace_zfs.h
@@ -1,24 +1 @@
-/* Here to keep the libspl build happy */
-
-#ifndef _LIBSPL_ZFS_TRACE_H
-#define	_LIBSPL_ZFS_TRACE_H
-
-/*
- * The set-error SDT probe is extra static, in that we declare its fake
- * function literally, rather than with the DTRACE_PROBE1() macro.  This is
- * necessary so that SET_ERROR() can evaluate to a value, which wouldn't
- * be possible if it required multiple statements (to declare the function
- * and then call it).
- *
- * SET_ERROR() uses the comma operator so that it can be used without much
- * additional code.  For example, "return (EINVAL);" becomes
- * "return (SET_ERROR(EINVAL));".  Note that the argument will be evaluated
- * twice, so it should not have side effects (e.g. something like:
- * "return (SET_ERROR(log_error(EINVAL, info)));" would log the error twice).
- */
-#undef SET_ERROR
-#define	SET_ERROR(err) \
-	(__set_error(__FILE__, __func__, __LINE__, err), err)
-
-
-#endif
+/* keep me */
diff --git a/lib/libzpool/include/sys/zfs_debug_os.h b/lib/libzpool/include/sys/zfs_debug_os.h
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: CDDL-1.0
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or https://opensource.org/licenses/CDDL-1.0.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+#ifndef _SYS_ZFS_DEBUG_OS_H
+#define	_SYS_ZFS_DEBUG_OS_H
+
+#define	SET_ERROR(err) \
+	(__set_error(__FILE__, __func__, __LINE__, err), err)
+
+#endif
diff --git a/module/os/freebsd/spl/spl_uio.c b/module/os/freebsd/spl/spl_uio.c
@@ -238,7 +238,7 @@ zfs_uio_iov_step(struct iovec v, zfs_uio_t *uio, int *numpages)
 	    zfs_uio_rw(uio), &uio->uio_dio.pages[uio->uio_dio.npages]);
 
 	if (res != n)
-		return (SET_ERROR(EFAULT));
+		return (EFAULT);
 
 	ASSERT3U(len, ==, res * PAGE_SIZE);
 	*numpages = res;
diff --git a/module/os/linux/spl/spl-taskq.c b/module/os/linux/spl/spl-taskq.c
@@ -32,7 +32,6 @@
 #include <sys/taskq.h>
 #include <sys/kmem.h>
 #include <sys/tsd.h>
-#include <sys/trace_spl.h>
 #include <sys/time.h>
 #include <sys/atomic.h>
 #include <sys/kstat.h>
@@ -325,7 +324,6 @@ task_expire_impl(taskq_ent_t *t)
 	}
 
 	t->tqent_birth = jiffies;
-	DTRACE_PROBE1(taskq_ent__birth, taskq_ent_t *, t);
 
 	/*
 	 * The priority list must be maintained in strict task id order
@@ -713,9 +711,7 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t flags)
 	t->tqent_taskq = tq;
 	t->tqent_timer.function = NULL;
 	t->tqent_timer.expires = 0;
-
 	t->tqent_birth = jiffies;
-	DTRACE_PROBE1(taskq_ent__birth, taskq_ent_t *, t);
 
 	ASSERT(!(t->tqent_flags & TQENT_FLAG_PREALLOC));
 
@@ -840,9 +836,7 @@ taskq_dispatch_ent(taskq_t *tq, task_func_t func, void *arg, uint_t flags,
 	t->tqent_func = func;
 	t->tqent_arg = arg;
 	t->tqent_taskq = tq;
-
 	t->tqent_birth = jiffies;
-	DTRACE_PROBE1(taskq_ent__birth, taskq_ent_t *, t);
 
 	spin_unlock(&t->tqent_lock);
 
@@ -1054,11 +1048,6 @@ taskq_thread(void *args)
 			 * A TQENT_FLAG_PREALLOC task may be reused or freed
 			 * during the task function call. Store tqent_id and
 			 * tqent_flags here.
-			 *
-			 * Also use an on stack taskq_ent_t for tqt_task
-			 * assignment in this case; we want to make sure
-			 * to duplicate all fields, so the values are
-			 * correct when it's accessed via DTRACE_PROBE*.
 			 */
 			tqt->tqt_id = t->tqent_id;
 			tqt->tqt_flags = t->tqent_flags;
@@ -1074,13 +1063,10 @@ taskq_thread(void *args)
 			spin_unlock_irqrestore(&tq->tq_lock, flags);
 
 			TQSTAT_INC(tq, threads_active);
-			DTRACE_PROBE1(taskq_ent__start, taskq_ent_t *, t);
 
 			/* Perform the requested task */
 			t->tqent_func(t->tqent_arg);
 
-			DTRACE_PROBE1(taskq_ent__finish, taskq_ent_t *, t);
-
 			TQSTAT_DEC(tq, threads_active);
 			if ((t->tqent_flags & TQENT_LIST_MASK) ==
 			    TQENT_LIST_PENDING)
diff --git a/scripts/spdxcheck.pl b/scripts/spdxcheck.pl
@@ -128,7 +128,6 @@
 	include/os/freebsd/zfs/sys/zpl.h
 	include/os/linux/kernel/linux/page_compat.h
 	lib/libspl/include/sys/string.h
-	lib/libspl/include/sys/trace_spl.h
 	lib/libzdb/libzdb.c
 	lib/libzpool/include/sys/trace_zfs.h
 	module/lua/setjmp/setjmp.S
