linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] input: Add tracepoint support
@ 2025-07-10  7:26 WangYuli
  2025-07-10  7:31 ` [PATCH 1/2] " WangYuli
  2025-07-10  7:31 ` [PATCH 2/2] selftests: input: Add tracepoint testing script WangYuli
  0 siblings, 2 replies; 8+ messages in thread
From: WangYuli @ 2025-07-10  7:26 UTC (permalink / raw)
  To: dmitry.torokhov, rostedt, mhiramat, mathieu.desnoyers, wangyuli
  Cc: linux-input, linux-kernel, linux-trace-kernel, zhanjun, niecheng1,
	guanwentao, wangyuli

Introduce a set of tracepoints for the input subsystem to enable
better debugging, monitoring, and performance analysis of input
device operations.

WangYuli (2):
  input: Add tracepoint support
  selftests: input: Add tracepoint testing script

 MAINTAINERS                                   |   3 +
 drivers/input/input.c                         |  18 +-
 include/trace/events/input.h                  | 251 ++++++++++++++++++
 .../selftests/input/test_input_tracepoints.sh |  78 ++++++
 4 files changed, 349 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/input.h
 create mode 100755 tools/testing/selftests/input/test_input_tracepoints.sh

-- 
2.50.0


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

* [PATCH 1/2] input: Add tracepoint support
  2025-07-10  7:26 [PATCH 0/2] input: Add tracepoint support WangYuli
@ 2025-07-10  7:31 ` WangYuli
  2025-07-23  0:25   ` Steven Rostedt
  2025-07-10  7:31 ` [PATCH 2/2] selftests: input: Add tracepoint testing script WangYuli
  1 sibling, 1 reply; 8+ messages in thread
From: WangYuli @ 2025-07-10  7:31 UTC (permalink / raw)
  To: wangyuli
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mathieu.desnoyers, mhiramat, niecheng1,
	rostedt, wangyuli, zhanjun, Winston Wen

Introduce a set of tracepoints for the input subsystem to enable
better debugging, monitoring, and performance analysis of input
device operations.

Suggested-by: Winston Wen <wentao@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 MAINTAINERS                  |   1 +
 drivers/input/input.c        |  18 ++-
 include/trace/events/input.h | 251 +++++++++++++++++++++++++++++++++++
 3 files changed, 269 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/input.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 00d7dcee88cb..c1b03679a5a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11933,6 +11933,7 @@ F:	include/linux/i8042.h
 F:	include/linux/input.h
 F:	include/linux/input/
 F:	include/linux/libps2.h
+F:	include/trace/events/input.h
 F:	include/linux/serio.h
 F:	include/uapi/linux/gameport.h
 F:	include/uapi/linux/input-event-codes.h
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 44887e51e049..925c76a50742 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -29,6 +29,9 @@
 #include "input-core-private.h"
 #include "input-poller.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/input.h>
+
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
 MODULE_LICENSE("GPL");
@@ -363,6 +366,7 @@ void input_handle_event(struct input_dev *dev,
 
 	disposition = input_get_disposition(dev, type, code, &value);
 	if (disposition != INPUT_IGNORE_EVENT) {
+		trace_input_event(dev, type, code, value);
 		if (type != EV_SYN)
 			add_input_randomness(type, code, value);
 
@@ -525,6 +529,7 @@ int input_grab_device(struct input_handle *handle)
 			return -EBUSY;
 
 		rcu_assign_pointer(dev->grab, handle);
+		trace_input_grab_device(dev, handle);
 	}
 
 	return 0;
@@ -539,6 +544,7 @@ static void __input_release_device(struct input_handle *handle)
 	grabber = rcu_dereference_protected(dev->grab,
 					    lockdep_is_held(&dev->mutex));
 	if (grabber == handle) {
+		trace_input_release_device(dev, handle);
 		rcu_assign_pointer(dev->grab, NULL);
 		/* Make sure input_pass_values() notices that grab is gone */
 		synchronize_rcu();
@@ -612,6 +618,7 @@ int input_open_device(struct input_handle *handle)
 
 		if (dev->poller)
 			input_dev_poller_start(dev->poller);
+		trace_input_open_device(dev);
 	}
 
 	return 0;
@@ -652,6 +659,7 @@ void input_close_device(struct input_handle *handle)
 				input_dev_poller_stop(dev->poller);
 			if (dev->close)
 				dev->close(dev);
+			trace_input_close_device(dev);
 		}
 	}
 
@@ -991,9 +999,13 @@ static int input_attach_handler(struct input_dev *dev, struct input_handler *han
 		return -ENODEV;
 
 	error = handler->connect(handler, dev, id);
-	if (error && error != -ENODEV)
+	if (error && error != -ENODEV) {
 		pr_err("failed to attach handler %s to device %s, error: %d\n",
 		       handler->name, kobject_name(&dev->dev.kobj), error);
+	} else if (!error) {
+		/* Connection successful, trace it */
+		trace_input_handler_connect(handler, dev, handler->minor);
+	}
 
 	return error;
 }
@@ -1791,6 +1803,7 @@ static int input_inhibit_device(struct input_dev *dev)
 	}
 
 	dev->inhibited = true;
+	trace_input_inhibit_device(dev);
 
 	return 0;
 }
@@ -1818,6 +1831,7 @@ static int input_uninhibit_device(struct input_dev *dev)
 
 	scoped_guard(spinlock_irq, &dev->event_lock)
 		input_dev_toggle(dev, true);
+	trace_input_uninhibit_device(dev);
 
 	return 0;
 }
@@ -2216,6 +2230,7 @@ static void __input_unregister_device(struct input_dev *dev)
 {
 	struct input_handle *handle, *next;
 
+	trace_input_device_unregister(dev);
 	input_disconnect_device(dev);
 
 	scoped_guard(mutex, &input_mutex) {
@@ -2414,6 +2429,7 @@ int input_register_device(struct input_dev *dev)
 		input_wakeup_procfs_readers();
 	}
 
+	trace_input_device_register(dev);
 	if (dev->devres_managed) {
 		dev_dbg(dev->dev.parent, "%s: registering %s with devres.\n",
 			__func__, dev_name(&dev->dev));
diff --git a/include/trace/events/input.h b/include/trace/events/input.h
new file mode 100644
index 000000000000..3c5ffcfb7c8d
--- /dev/null
+++ b/include/trace/events/input.h
@@ -0,0 +1,251 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* input tracepoints
+ *
+ * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM input
+
+#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INPUT_H
+
+#include <linux/tracepoint.h>
+#include <linux/input.h>
+
+/**
+ * input_event - called when an input event is processed
+ * @dev: input device that generated the event
+ * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
+ * @code: event code within the type
+ * @value: event value
+ *
+ * This tracepoint fires for every input event processed by the input core.
+ * It can be used to monitor input device activity and debug input issues.
+ */
+TRACE_EVENT(
+	input_event,
+
+	TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
+		 int value),
+
+	TP_ARGS(dev, type, code, value),
+
+	TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
+		unsigned int, type) __field(unsigned int, code)
+				 __field(int, value) __field(u16, bustype)
+					 __field(u16, vendor)
+						 __field(u16, product)),
+
+	TP_fast_assign(__assign_str(name); __entry->type = type;
+		       __entry->code = code; __entry->value = value;
+		       __entry->bustype = dev->id.bustype;
+		       __entry->vendor = dev->id.vendor;
+		       __entry->product = dev->id.product;),
+
+	TP_printk(
+		"dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
+		__get_str(name), __entry->type, __entry->code, __entry->value,
+		__entry->bustype, __entry->vendor, __entry->product));
+
+/**
+ * input_device_register - called when an input device is registered
+ * @dev: input device being registered
+ *
+ * This tracepoint fires when a new input device is registered with the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(
+	input_device_register,
+
+	TP_PROTO(struct input_dev *dev),
+
+	TP_ARGS(dev),
+
+	TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
+		phys, dev->phys ?: "") __field(u16, bustype)
+				 __field(u16, vendor) __field(u16, product)
+					 __field(u16, version)),
+
+	TP_fast_assign(__assign_str(name); __assign_str(phys);
+		       __entry->bustype = dev->id.bustype;
+		       __entry->vendor = dev->id.vendor;
+		       __entry->product = dev->id.product;
+		       __entry->version = dev->id.version;),
+
+	TP_printk(
+		"dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
+		__get_str(name), __get_str(phys), __entry->bustype,
+		__entry->vendor, __entry->product, __entry->version));
+
+/**
+ * input_device_unregister - called when an input device is unregistered
+ * @dev: input device being unregistered
+ *
+ * This tracepoint fires when an input device is unregistered from the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(input_device_unregister,
+
+	    TP_PROTO(struct input_dev *dev),
+
+	    TP_ARGS(dev),
+
+	    TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
+		    phys, dev->phys ?: "") __field(u16, bustype)
+				     __field(u16, vendor)
+					     __field(u16, product)),
+
+	    TP_fast_assign(__assign_str(name); __assign_str(phys);
+			   __entry->bustype = dev->id.bustype;
+			   __entry->vendor = dev->id.vendor;
+			   __entry->product = dev->id.product;),
+
+	    TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04x",
+		      __get_str(name), __get_str(phys), __entry->bustype,
+		      __entry->vendor, __entry->product));
+
+/**
+ * input_handler_connect - called when an input handler connects to a device
+ * @handler: input handler
+ * @dev: input device
+ * @minor: assigned minor number (if applicable)
+ *
+ * This tracepoint fires when an input handler (like evdev, mousedev) connects
+ * to an input device, creating a new input handle.
+ */
+TRACE_EVENT(input_handler_connect,
+
+	    TP_PROTO(struct input_handler *handler, struct input_dev *dev,
+		     int minor),
+
+	    TP_ARGS(handler, dev, minor),
+
+	    TP_STRUCT__entry(__string(handler_name, handler->name)
+				     __string(dev_name, dev->name ?: "unknown")
+					     __field(int, minor)),
+
+	    TP_fast_assign(__assign_str(handler_name); __assign_str(dev_name);
+			   __entry->minor = minor;),
+
+	    TP_printk("handler=%s dev=%s minor=%d", __get_str(handler_name),
+		      __get_str(dev_name), __entry->minor));
+
+/**
+ * input_grab_device - called when a device is grabbed by a handler
+ * @dev: input device being grabbed
+ * @handle: input handle doing the grab
+ *
+ * This tracepoint fires when an input device is grabbed exclusively
+ * by an input handler, typically for security or special processing.
+ */
+TRACE_EVENT(input_grab_device,
+
+	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+	    TP_ARGS(dev, handle),
+
+	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+				     __string(handler_name,
+					      handle->handler->name)),
+
+	    TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
+
+	    TP_printk("dev=%s grabbed_by=%s", __get_str(dev_name),
+		      __get_str(handler_name)));
+
+/**
+ * input_release_device - called when a grabbed device is released
+ * @dev: input device being released
+ * @handle: input handle releasing the grab
+ *
+ * This tracepoint fires when an input device grab is released.
+ */
+TRACE_EVENT(input_release_device,
+
+	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+	    TP_ARGS(dev, handle),
+
+	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+				     __string(handler_name,
+					      handle->handler->name)),
+
+	    TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
+
+	    TP_printk("dev=%s released_by=%s", __get_str(dev_name),
+		      __get_str(handler_name)));
+
+DECLARE_EVENT_CLASS(input_device_state,
+
+		    TP_PROTO(struct input_dev *dev),
+
+		    TP_ARGS(dev),
+
+		    TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+					     __field(unsigned int, users)
+						     __field(bool, inhibited)),
+
+		    TP_fast_assign(__assign_str(name);
+				   __entry->users = dev->users;
+				   __entry->inhibited = dev->inhibited;),
+
+		    TP_printk("dev=%s users=%u inhibited=%s", __get_str(name),
+			      __entry->users,
+			      __entry->inhibited ? "true" : "false"));
+
+/**
+ * input_open_device - called when an input device is opened
+ * @dev: input device being opened
+ *
+ * This tracepoint fires when the user count for an input device increases,
+ * typically when a userspace application opens the device.
+ */
+DEFINE_EVENT(input_device_state, input_open_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_close_device - called when an input device is closed
+ * @dev: input device being closed
+ *
+ * This tracepoint fires when the user count for an input device decreases,
+ * typically when a userspace application closes the device.
+ */
+DEFINE_EVENT(input_device_state, input_close_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_inhibit_device - called when an input device is inhibited
+ * @dev: input device being inhibited
+ *
+ * This tracepoint fires when an input device is inhibited (disabled),
+ * usually for power management or security reasons.
+ */
+DEFINE_EVENT(input_device_state, input_inhibit_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_uninhibit_device - called when an input device is uninhibited
+ * @dev: input device being uninhibited
+ *
+ * This tracepoint fires when an input device is uninhibited (re-enabled)
+ * after being previously inhibited.
+ */
+DEFINE_EVENT(input_device_state, input_uninhibit_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+#endif /* _TRACE_INPUT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.50.0


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

* [PATCH 2/2] selftests: input: Add tracepoint testing script
  2025-07-10  7:26 [PATCH 0/2] input: Add tracepoint support WangYuli
  2025-07-10  7:31 ` [PATCH 1/2] " WangYuli
@ 2025-07-10  7:31 ` WangYuli
  1 sibling, 0 replies; 8+ messages in thread
From: WangYuli @ 2025-07-10  7:31 UTC (permalink / raw)
  To: wangyuli
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mathieu.desnoyers, mhiramat, niecheng1,
	rostedt, wangyuli, zhanjun

Add a test script for the input subsystem tracepoints.

The script provides:
 - Automatic detection of available input tracepoints
 - Privilege and filesystem mount validation
 - Interactive testing with real-time trace monitoring
 - Support for both batch and live trace analysis

The script enables easy validation and demonstration of the input
tracepoint functionality, making it easier for developers to verify
the implementation and understand the tracepoint capabilities.

Usage:
  sudo ./tools/testing/selftests/input/test_input_tracepoints.sh

Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
 MAINTAINERS                                   |  2 +
 .../selftests/input/test_input_tracepoints.sh | 78 +++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100755 tools/testing/selftests/input/test_input_tracepoints.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index c1b03679a5a1..8a0920734303 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11940,6 +11940,7 @@ F:	include/uapi/linux/input-event-codes.h
 F:	include/uapi/linux/input.h
 F:	include/uapi/linux/serio.h
 F:	include/uapi/linux/uinput.h
+F:	tools/testing/selftests/input/
 
 INPUT MULTITOUCH (MT) PROTOCOL
 M:	Henrik Rydberg <rydberg@bitmath.org>
@@ -25211,6 +25212,7 @@ F:	kernel/trace/
 F:	kernel/tracepoint.c
 F:	scripts/tracing/
 F:	tools/testing/selftests/ftrace/
+F:	tools/testing/selftests/input/test_input_tracepoints.sh
 
 TRACING MMIO ACCESSES (MMIOTRACE)
 M:	Steven Rostedt <rostedt@goodmis.org>
diff --git a/tools/testing/selftests/input/test_input_tracepoints.sh b/tools/testing/selftests/input/test_input_tracepoints.sh
new file mode 100755
index 000000000000..6ade2619b62d
--- /dev/null
+++ b/tools/testing/selftests/input/test_input_tracepoints.sh
@@ -0,0 +1,78 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+###############################################################################
+#
+# Input subsystem tracepoint testing script
+#
+# AUTHOR
+#      WangYuli <wangyuli@uniontech.com>
+#
+###############################################################################
+
+DEBUGFS_PATH="/sys/kernel/debug/tracing"
+INPUT_EVENTS_PATH="${DEBUGFS_PATH}/events/input"
+
+# Check if we have sufficient privileges
+if [ ! -w "$DEBUGFS_PATH" ]; then
+    echo "Error: Root privileges required to access tracing system"
+    echo "Please run: sudo $0"
+    exit 1
+fi
+
+# Check if debugfs is mounted
+if [ ! -d "$DEBUGFS_PATH" ]; then
+    echo "Error: debugfs is not mounted"
+    echo "Please run: mount -t debugfs none /sys/kernel/debug"
+    exit 1
+fi
+
+# Check if input tracepoints exist
+if [ ! -d "$INPUT_EVENTS_PATH" ]; then
+    echo "Error: input tracepoints not found, kernel may need to be recompiled"
+    exit 1
+fi
+
+echo "=== Input Subsystem Tracepoint Test ==="
+echo
+
+# Clear existing trace buffer
+echo > "${DEBUGFS_PATH}/trace"
+
+# List available input tracepoints
+echo "Available Input Tracepoints:"
+for event in "${INPUT_EVENTS_PATH}"/*; do
+    if [ -d "$event" ]; then
+        event_name=$(basename "$event")
+        echo "  - $event_name"
+    fi
+done
+echo
+
+# Enable all input tracepoints
+echo "Enabling all input tracepoints..."
+echo 1 > "${INPUT_EVENTS_PATH}/enable"
+
+if [ $? -eq 0 ]; then
+    echo "✓ Successfully enabled input tracepoints"
+else
+    echo "✗ Failed to enable input tracepoints"
+    exit 1
+fi
+
+echo
+echo "Please perform some operations in another terminal (keyboard input, mouse movement, etc.)"
+echo "or plug/unplug USB devices, then come back to view the results..."
+echo
+echo "Press any key to continue viewing trace output (press Ctrl+C to exit)..."
+read -n 1
+
+echo
+echo "=== Trace Output ==="
+echo "(Last 100 lines)"
+tail -n 100 "${DEBUGFS_PATH}/trace"
+
+echo
+echo "=== Real-time Trace Output ==="
+echo "(Press Ctrl+C to stop)"
+cat "${DEBUGFS_PATH}/trace_pipe"
-- 
2.50.0


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

* Re: [PATCH 1/2] input: Add tracepoint support
  2025-07-10  7:31 ` [PATCH 1/2] " WangYuli
@ 2025-07-23  0:25   ` Steven Rostedt
  2025-07-23  1:24     ` Mathieu Desnoyers
  2025-07-28  6:51     ` WangYuli
  0 siblings, 2 replies; 8+ messages in thread
From: Steven Rostedt @ 2025-07-23  0:25 UTC (permalink / raw)
  To: WangYuli
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mathieu.desnoyers, mhiramat, niecheng1,
	wangyuli, zhanjun, Winston Wen

On Thu, 10 Jul 2025 15:31:38 +0800
WangYuli <wangyuli@uniontech.com> wrote:

> diff --git a/include/trace/events/input.h b/include/trace/events/input.h
> new file mode 100644
> index 000000000000..3c5ffcfb7c8d
> --- /dev/null
> +++ b/include/trace/events/input.h
> @@ -0,0 +1,251 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* input tracepoints
> + *
> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
> + */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM input
> +
> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_INPUT_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/input.h>
> +
> +/**
> + * input_event - called when an input event is processed
> + * @dev: input device that generated the event
> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
> + * @code: event code within the type
> + * @value: event value
> + *
> + * This tracepoint fires for every input event processed by the input core.
> + * It can be used to monitor input device activity and debug input issues.
> + */
> +TRACE_EVENT(
> +	input_event,
> +
> +	TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
> +		 int value),
> +
> +	TP_ARGS(dev, type, code, value),
> +
> +	TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
> +		unsigned int, type) __field(unsigned int, code)
> +				 __field(int, value) __field(u16, bustype)
> +					 __field(u16, vendor)
> +						 __field(u16, product)),
> +

The contents of the tracepoints in the subsystems are determined by the
subsystem maintainers, but please follow the tracepoint formatting. The
above is horrible. It should look like a structure layout. One wouldn't
write:

struct entry { char *name;
		unsigned int type; unsigned int code;
			int value; u16 bustype;
				u16 vendor;
					u16 product; };

That's what the above looks like. It should be instead:

	TP_STRUCT__entry(
		__string(	name,		dev->name ?: "unknown"	)
		__field(	unsigned int,	type			)
		__field(	unsigned int,	code			)
		__field(	int,		value			)
		__field(	u16,		bustype			)
		__field(	u16,		vendor			)
		__field(	u16,		product			)
	),

So the fields can be easily visible and easily reviewed.


> +	TP_fast_assign(__assign_str(name); __entry->type = type;
> +		       __entry->code = code; __entry->value = value;
> +		       __entry->bustype = dev->id.bustype;
> +		       __entry->vendor = dev->id.vendor;
> +		       __entry->product = dev->id.product;),
> +
> +	TP_printk(
> +		"dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
> +		__get_str(name), __entry->type, __entry->code, __entry->value,
> +		__entry->bustype, __entry->vendor, __entry->product));
> +
> +/**
> + * input_device_register - called when an input device is registered
> + * @dev: input device being registered
> + *
> + * This tracepoint fires when a new input device is registered with the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(
> +	input_device_register,
> +
> +	TP_PROTO(struct input_dev *dev),
> +
> +	TP_ARGS(dev),
> +
> +	TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> +		phys, dev->phys ?: "") __field(u16, bustype)
> +				 __field(u16, vendor) __field(u16, product)
> +					 __field(u16, version)),

Same here.

> +
> +	TP_fast_assign(__assign_str(name); __assign_str(phys);
> +		       __entry->bustype = dev->id.bustype;
> +		       __entry->vendor = dev->id.vendor;
> +		       __entry->product = dev->id.product;
> +		       __entry->version = dev->id.version;),
> +
> +	TP_printk(
> +		"dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
> +		__get_str(name), __get_str(phys), __entry->bustype,
> +		__entry->vendor, __entry->product, __entry->version));
> +
> +/**
> + * input_device_unregister - called when an input device is unregistered
> + * @dev: input device being unregistered
> + *
> + * This tracepoint fires when an input device is unregistered from the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(input_device_unregister,
> +
> +	    TP_PROTO(struct input_dev *dev),
> +
> +	    TP_ARGS(dev),
> +
> +	    TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> +		    phys, dev->phys ?: "") __field(u16, bustype)
> +				     __field(u16, vendor)
> +					     __field(u16, product)),

ditto.

> +
> +	    TP_fast_assign(__assign_str(name); __assign_str(phys);
> +			   __entry->bustype = dev->id.bustype;
> +			   __entry->vendor = dev->id.vendor;
> +			   __entry->product = dev->id.product;),
> +
> +	    TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04x",
> +		      __get_str(name), __get_str(phys), __entry->bustype,
> +		      __entry->vendor, __entry->product));
> +
> +/**
> + * input_handler_connect - called when an input handler connects to a device
> + * @handler: input handler
> + * @dev: input device
> + * @minor: assigned minor number (if applicable)
> + *
> + * This tracepoint fires when an input handler (like evdev, mousedev) connects
> + * to an input device, creating a new input handle.
> + */
> +TRACE_EVENT(input_handler_connect,
> +
> +	    TP_PROTO(struct input_handler *handler, struct input_dev *dev,
> +		     int minor),
> +
> +	    TP_ARGS(handler, dev, minor),
> +
> +	    TP_STRUCT__entry(__string(handler_name, handler->name)
> +				     __string(dev_name, dev->name ?: "unknown")
> +					     __field(int, minor)),

ditto.

> +
> +	    TP_fast_assign(__assign_str(handler_name); __assign_str(dev_name);
> +			   __entry->minor = minor;),
> +
> +	    TP_printk("handler=%s dev=%s minor=%d", __get_str(handler_name),
> +		      __get_str(dev_name), __entry->minor));
> +
> +/**
> + * input_grab_device - called when a device is grabbed by a handler
> + * @dev: input device being grabbed
> + * @handle: input handle doing the grab
> + *
> + * This tracepoint fires when an input device is grabbed exclusively
> + * by an input handler, typically for security or special processing.
> + */
> +TRACE_EVENT(input_grab_device,
> +
> +	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> +	    TP_ARGS(dev, handle),
> +
> +	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> +				     __string(handler_name,
> +					      handle->handler->name)),

ditto.

> +
> +	    TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> +	    TP_printk("dev=%s grabbed_by=%s", __get_str(dev_name),
> +		      __get_str(handler_name)));
> +
> +/**
> + * input_release_device - called when a grabbed device is released
> + * @dev: input device being released
> + * @handle: input handle releasing the grab
> + *
> + * This tracepoint fires when an input device grab is released.
> + */
> +TRACE_EVENT(input_release_device,
> +
> +	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> +	    TP_ARGS(dev, handle),
> +
> +	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> +				     __string(handler_name,
> +					      handle->handler->name)),

ditto.

> +
> +	    TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> +	    TP_printk("dev=%s released_by=%s", __get_str(dev_name),
> +		      __get_str(handler_name)));
> +
> +DECLARE_EVENT_CLASS(input_device_state,
> +
> +		    TP_PROTO(struct input_dev *dev),
> +
> +		    TP_ARGS(dev),
> +
> +		    TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
> +					     __field(unsigned int, users)
> +						     __field(bool, inhibited)),

ditto.

-- Steve

> +
> +		    TP_fast_assign(__assign_str(name);
> +				   __entry->users = dev->users;
> +				   __entry->inhibited = dev->inhibited;),
> +
> +		    TP_printk("dev=%s users=%u inhibited=%s", __get_str(name),
> +			      __entry->users,
> +			      __entry->inhibited ? "true" : "false"));
> +

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

* Re: [PATCH 1/2] input: Add tracepoint support
  2025-07-23  0:25   ` Steven Rostedt
@ 2025-07-23  1:24     ` Mathieu Desnoyers
  2025-07-28  7:07       ` WangYuli
  2025-07-28  6:51     ` WangYuli
  1 sibling, 1 reply; 8+ messages in thread
From: Mathieu Desnoyers @ 2025-07-23  1:24 UTC (permalink / raw)
  To: Steven Rostedt, WangYuli
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mhiramat, niecheng1, wangyuli, zhanjun,
	Winston Wen

On 2025-07-22 20:25, Steven Rostedt wrote:
> On Thu, 10 Jul 2025 15:31:38 +0800
> WangYuli <wangyuli@uniontech.com> wrote:
> 
>> diff --git a/include/trace/events/input.h b/include/trace/events/input.h
>> new file mode 100644
>> index 000000000000..3c5ffcfb7c8d
>> --- /dev/null
>> +++ b/include/trace/events/input.h
>> @@ -0,0 +1,251 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* input tracepoints
>> + *
>> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
>> + */
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM input
>> +
>> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_INPUT_H
>> +
>> +#include <linux/tracepoint.h>
>> +#include <linux/input.h>
>> +
>> +/**
>> + * input_event - called when an input event is processed
>> + * @dev: input device that generated the event
>> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
>> + * @code: event code within the type
>> + * @value: event value
>> + *
>> + * This tracepoint fires for every input event processed by the input core.
>> + * It can be used to monitor input device activity and debug input issues.
>> + */

I've always been worried about adding tracepoint instrumentation of the
input subsystem that includes the actual keystrokes into the event
payload. What I'm trying to avoid here is people leaking their password
by mistake just because they happened to record a trace while
typing on their keyboard.

I don't mind if this gets enabled with a new kernel command line
options "tracing_leak_my_credentials=yes" or such, but I'd try to
avoid making it easy to enable by mistake unless this information
is specifically needed.

But maybe I'm being too careful and people should really learn not
to share kernel traces with others.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

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

* Re: [PATCH 1/2] input: Add tracepoint support
  2025-07-23  0:25   ` Steven Rostedt
  2025-07-23  1:24     ` Mathieu Desnoyers
@ 2025-07-28  6:51     ` WangYuli
  1 sibling, 0 replies; 8+ messages in thread
From: WangYuli @ 2025-07-28  6:51 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mathieu.desnoyers, mhiramat, niecheng1,
	wangyuli, zhanjun, Winston Wen


[-- Attachment #1.1.1: Type: text/plain, Size: 1308 bytes --]

Hi Steve,

On 2025/7/23 08:25, Steven Rostedt wrote:
>> +	TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
>> +		unsigned int, type) __field(unsigned int, code)
>> +				 __field(int, value) __field(u16, bustype)
>> +					 __field(u16, vendor)
>> +						 __field(u16, product)),
>> +
> The contents of the tracepoints in the subsystems are determined by the
> subsystem maintainers, but please follow the tracepoint formatting. The
> above is horrible. It should look like a structure layout. One wouldn't
> write:
>
> struct entry { char *name;
> 		unsigned int type; unsigned int code;
> 			int value; u16 bustype;
> 				u16 vendor;
> 					u16 product; };
>
> That's what the above looks like. It should be instead:
>
> 	TP_STRUCT__entry(
> 		__string(	name,		dev->name ?: "unknown"	)
> 		__field(	unsigned int,	type			)
> 		__field(	unsigned int,	code			)
> 		__field(	int,		value			)
> 		__field(	u16,		bustype			)
> 		__field(	u16,		vendor			)
> 		__field(	u16,		product			)
> 	),
>
> So the fields can be easily visible and easily reviewed.

My apologies.

Since this was a new file I added, I didn't carefully check it after 
applying clang-format, which led to this issue.

I'll fix the code formatting and send a patch v2.

-- 
WangYuli*
*

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 645 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

* Re: [PATCH 1/2] input: Add tracepoint support
  2025-07-23  1:24     ` Mathieu Desnoyers
@ 2025-07-28  7:07       ` WangYuli
  2025-07-28 13:53         ` Mathieu Desnoyers
  0 siblings, 1 reply; 8+ messages in thread
From: WangYuli @ 2025-07-28  7:07 UTC (permalink / raw)
  To: Mathieu Desnoyers, Steven Rostedt
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mhiramat, niecheng1, wangyuli, zhanjun,
	Winston Wen


[-- Attachment #1.1.1: Type: text/plain, Size: 1490 bytes --]

Hi Mathieu,

On 2025/7/23 09:24, Mathieu Desnoyers wrote:
> I've always been worried about adding tracepoint instrumentation of the
> input subsystem that includes the actual keystrokes into the event
> payload. What I'm trying to avoid here is people leaking their password
> by mistake just because they happened to record a trace while
> typing on their keyboard.
>
The evtest tool can also do this.

However, it doesn't fully report all events from the input subsystem.

 From a debugging perspective, adding tracepoints to the input subsystem 
is still more convenient for debugging.

> I don't mind if this gets enabled with a new kernel command line
> options "tracing_leak_my_credentials=yes" or such, but I'd try to
> avoid making it easy to enable by mistake unless this information
> is specifically needed.
>
I'm not sure if this is over-engineering...

I feel that adding too many command-line parameters will increase the 
user's cognitive load.

However, the leakage of keyboard input records is indeed a very, very 
significant risk.

As a compromise, would it be better if we added a separate Kconfig 
option specifically for the input subsystem's tracepoints to decide 
whether to enable them at compile time, and then documented the 
potential risks within that Kconfig's description?
> But maybe I'm being too careful and people should really learn not
> to share kernel traces with others.
>
> Thoughts ?
>
Thanks,
-- 
WangYuli

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 645 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

* Re: [PATCH 1/2] input: Add tracepoint support
  2025-07-28  7:07       ` WangYuli
@ 2025-07-28 13:53         ` Mathieu Desnoyers
  0 siblings, 0 replies; 8+ messages in thread
From: Mathieu Desnoyers @ 2025-07-28 13:53 UTC (permalink / raw)
  To: WangYuli, Steven Rostedt
  Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
	linux-trace-kernel, mhiramat, niecheng1, wangyuli, zhanjun,
	Winston Wen

On 2025-07-28 03:07, WangYuli wrote:
> Hi Mathieu,
> 
> On 2025/7/23 09:24, Mathieu Desnoyers wrote:
>> I've always been worried about adding tracepoint instrumentation of the
>> input subsystem that includes the actual keystrokes into the event
>> payload. What I'm trying to avoid here is people leaking their password
>> by mistake just because they happened to record a trace while
>> typing on their keyboard.
>>
> The evtest tool can also do this.
> 
> However, it doesn't fully report all events from the input subsystem.
> 
>  From a debugging perspective, adding tracepoints to the input subsystem 
> is still more convenient for debugging.
> 
>> I don't mind if this gets enabled with a new kernel command line
>> options "tracing_leak_my_credentials=yes" or such, but I'd try to
>> avoid making it easy to enable by mistake unless this information
>> is specifically needed.
>>
> I'm not sure if this is over-engineering...
> 
> I feel that adding too many command-line parameters will increase the 
> user's cognitive load.
> 
> However, the leakage of keyboard input records is indeed a very, very 
> significant risk.
> 
> As a compromise, would it be better if we added a separate Kconfig 
> option specifically for the input subsystem's tracepoints to decide 
> whether to enable them at compile time, and then documented the 
> potential risks within that Kconfig's description?

In term of mechanism to select keylogger enabling/disabling, I can
think of the following options:

- Kconfig option,
- kernel command line parameter,
- sysctl

Here is a possibly incomplete list of desiderata for this:

- Keylogger should be disabled by default.
- System administrator should be able to enable keylogger at boot.
- Users should be able to query the state of keylogger
   (enabled/disabled) during kernel execution, and this should be
   invariant until reboot,
- Selecting whether this option is enabled or not should be decided
   by the system administrator, not by the distribution vendors who
   compile the distro kernels.
- Prevent use of a kernel tracer as a keylogger by mistake without
   having the system administrator explicitly enable keylogging.

The most flexible approach would be a sysctl, because it would allow
a system administrator to enable this while the system runs. But it
is somewhat redundant with the fact that the tracers allow disabling
specific events dynamically. Also I don't think we would want to allow
changing this configuration after system boots, so users interacting
with a production system can check whether this is enabled or not to
learn whether they can trust that this keylogger feature is disabled.

This leaves Kconfig option and kernel command line. The downside of the
Kconfig option is that it requires to choose the configuration up
front for a distro kernel, not allowing the system admin to select
the behavior at boot time without recompiling a custom kernel.

This leaves the kernel command line option, which I think is a
good tradeoff. It allows sysadmins to enable keylogging at boot
from a distro kernel without recompiling their own kernel. It
also prevents enabling keylogging in a production system by
mistake after bootup. It allows users to inspect the status of
this knob by looking at the kernel command line to know whether
they are interacting with a system that has this keylogging
enabled.

Thoughts ?

Thanks,

Mathieu

>> But maybe I'm being too careful and people should really learn not
>> to share kernel traces with others.
>>
>> Thoughts ?
>>
> Thanks,


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

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

end of thread, other threads:[~2025-07-28 13:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10  7:26 [PATCH 0/2] input: Add tracepoint support WangYuli
2025-07-10  7:31 ` [PATCH 1/2] " WangYuli
2025-07-23  0:25   ` Steven Rostedt
2025-07-23  1:24     ` Mathieu Desnoyers
2025-07-28  7:07       ` WangYuli
2025-07-28 13:53         ` Mathieu Desnoyers
2025-07-28  6:51     ` WangYuli
2025-07-10  7:31 ` [PATCH 2/2] selftests: input: Add tracepoint testing script WangYuli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).