From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9417C6FD18 for ; Tue, 28 Mar 2023 20:59:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 342066B0074; Tue, 28 Mar 2023 16:59:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F277900002; Tue, 28 Mar 2023 16:59:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E14D6B0078; Tue, 28 Mar 2023 16:59:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 104A56B0074 for ; Tue, 28 Mar 2023 16:59:47 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DF6BB14038D for ; Tue, 28 Mar 2023 20:59:46 +0000 (UTC) X-FDA: 80619523572.28.DF09056 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id 3E871140018 for ; Tue, 28 Mar 2023 20:59:45 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf26.hostedemail.com: domain of "SRS0=rtO5=7U=goodmis.org=rostedt@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=rtO5=7U=goodmis.org=rostedt@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680037185; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=k0LfW04zr1SLofpFWsAXviJn+a63cJkhqaGlxedDKHU=; b=ZEtszzZlfx6GUOHTpcvuyi8nmMaRYzh5negkh3+RQ7Pq0kWPtXMjI3wumWc+6KGL2uEbES 3gWYYMnEHaP3WG2SvFkkdxn6lYpUcUe0sSKuIEIQkQ01j/tKCaxQ/7lwDp2u45A5JNVqm3 DIB5ewnQGVUh60R1WTNCcZ/QaSKKIWM= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf26.hostedemail.com: domain of "SRS0=rtO5=7U=goodmis.org=rostedt@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=rtO5=7U=goodmis.org=rostedt@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680037185; a=rsa-sha256; cv=none; b=taahOgut30wP0haIlWSAO+ER7a3iKk5UPgtH+v8k/i0pQ9M+N134Adj8BrwxqbRuXdxxxN pEOv3p0IXs9x0rwJn0uRHTlZDbN1vH5uoLFgPdnt3QPBoM4LFHmnu7CyA+uqAXE0QJZLC9 Aa9e+kC7lepsCdaNWy0fZNCVrE2yX6Q= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 312A76195D; Tue, 28 Mar 2023 20:59:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 31D5AC433EF; Tue, 28 Mar 2023 20:59:42 +0000 (UTC) Date: Tue, 28 Mar 2023 16:59:39 -0400 From: Steven Rostedt To: Beau Belgrave Cc: mhiramat@kernel.org, mathieu.desnoyers@efficios.com, dcook@linux.microsoft.com, alanau@linux.microsoft.com, brauner@kernel.org, akpm@linux-foundation.org, ebiederm@xmission.com, keescook@chromium.org, tglx@linutronix.de, linux-trace-devel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v8 03/11] tracing/user_events: Use remote writes for event enablement Message-ID: <20230328165939.1dcddd1a@gandalf.local.home> In-Reply-To: <20230221211143.574-4-beaub@linux.microsoft.com> References: <20230221211143.574-1-beaub@linux.microsoft.com> <20230221211143.574-4-beaub@linux.microsoft.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 3E871140018 X-Stat-Signature: 4idh6bymip4qregtqfe9rddoo1suzmuz X-HE-Tag: 1680037185-215379 X-HE-Meta: U2FsdGVkX1+WEIJkmIUmcMUaQtnBtLtEyBLZNAbQZv3odBwx6hcE5u93VfGxGFQgPv+U98z85sfgR7ROMG2OfnB/ZtXaT2xpy0/uAEnllBC7Kl1PxywVdwjUicwioSFo/Y4R8OVoFOc3fieN61RUyG7+LwJa6YIFltuoYUQUB1EsKPt9uzb3YJvcGdAhllPHp/WpfY6d69GAgki/KDtrygr7HVrOXh0nmYiK8FHnvvgpxbanNZnoRpDAw8DZMYVYfU3XzbMY1AuOWuYRAZA16CGm0Q7oXNZynzHy/1jNBUctNSZCyLOFbBZz5EJf2j16Z3uFUjwXCbiyqVvMqWiYWbwHEwmKEnQJCkWB9xuMClHY48aFNO5LsRT7+0qR8urvlqfrfIXeHOVteW3s9qnB2nWiRH+arnATVEad7P7owXmmW0rUQB8PUzD4lSthNj/iz5YOPyUsfSm08BOMtjo1dMWvJ0tbJiCruuSn4kr0ZgdawBWOrs3kqXTYUtbe/LyH730Gci9EcSfHxYyIYUtWxY1D+WUlLO7gx604Q4G0Q+kzczbwM/sH8BwhRZtcYA/NA9qvac3VXYG2weWWQ9hSyi+1CoOLWnJhK+mECHYKR1YhTdn47g+hysljklwgVik3bia9W8o7iUWQeD+ZgaQF9xzyVjD1kz3LMqfOZSkUwHowBMhJ3skEhn2ohkUcu/mx2clLplCXihGrv4vu+eV+iDTdfkvsZCnMlyfkRLHQEplveB3ARtBfIiDMyav2PvyAaoiTQlXQK95nZtpimyZCMPU0bSyg8X5dB9+jlzAv6YAT6bNAfvEpRMEUkejUch5nvPXFcCiZZSG8ONyX4xq2wvvi/XfgXtPZ0st1wIw25k+aef4EOUuj9vtLigyGrdOuQpEHqgm3uzY61CSUIa6Dgl9MIMiuv56Ts2cpnzPaXPDS79V7g+rchwcm+w4UfRrFS2jGucjF6KVNGIELM/X eDo7WvG4 LEcpxKakKIaq3bpu4TlQ6CDU6z1VpkyRQOrx7k8agqXqGa0bhvHbDqv1v5w0cU6QVQzPh1KYkzbQOAHPVFtte0ub0E9PWwuH+8v2TVsIDCU9SzH7b206rSRsCfkBYjFuw2y4cL11NlZt1/lELKeU31VdQDtGZAd8aAM06jhHV3Zwbj8TFnym27yuZe8kY6qjOLbcJKVQGluC9BTAG0+/7UKqQq1sTDvz6/kqcEeTWKRZIETOBMheRMOUst9SGqJNac/CQAxbjokdZqa5J6WLYLU//H7iFvdXyCq1AEXfzIKOhqc2RGg1Ja51mqdmmCoeh1Y5zzY4A3mM6DjmTJT2qYT2kD0GVVJzLTImdS7q50eXr1RIxeyzt63xKP9MJpcaB6QYNymlQv0+inXw68Gm1Rz3p+iUjcua63h6CMg7QsD5AawV7uvbKjJb7H46OBqUFtv3d X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 21 Feb 2023 13:11:35 -0800 Beau Belgrave wrote: > include/linux/user_events.h | 53 ++- > include/uapi/linux/user_events.h | 15 +- > kernel/trace/Kconfig | 5 +- > kernel/trace/trace_events_user.c | 586 ++++++++++++++++++++++++------- > 4 files changed, 517 insertions(+), 142 deletions(-) > > diff --git a/include/linux/user_events.h b/include/linux/user_events.h > index 3d747c45d2fa..0120b3dd5b03 100644 > --- a/include/linux/user_events.h > +++ b/include/linux/user_events.h > @@ -9,13 +9,63 @@ > #ifndef _LINUX_USER_EVENTS_H > #define _LINUX_USER_EVENTS_H > > +#include > +#include > +#include > +#include > #include > > #ifdef CONFIG_USER_EVENTS > struct user_event_mm { > + struct list_head link; > + struct list_head enablers; > + struct mm_struct *mm; > + struct user_event_mm *next; > + refcount_t refcnt; > + refcount_t tasks; > + struct rcu_work put_rwork; > }; This is more of a nit, and not something to change unless there's more real content to change, but please when making structures tab out the field names: struct user_event_mm { struct list_head link; struct list_head enablers; struct mm_struct *mm; struct user_event_mm *next; refcount_t refcnt; refcount_t tasks; struct rcu_work put_rwork; }; See how much easier it is to read (and know what fields exist). > -#endif > > #endif /* _LINUX_USER_EVENTS_H */ > diff --git a/include/uapi/linux/user_events.h b/include/uapi/linux/user_events.h > index 03f92366068d..22521bc622db 100644 > --- a/include/uapi/linux/user_events.h > +++ b/include/uapi/linux/user_events.h > @@ -27,12 +27,21 @@ struct user_reg { > /* Input: Size of the user_reg structure being used */ > __u32 size; > > + /* Input: Bit in enable address to use */ > + __u8 enable_bit; > + > + /* Input: Enable size in bytes at address */ > + __u8 enable_size; > + > + /* Input: Flags for future use, set to 0 */ > + __u16 flags; > + > + /* Input: Address to update when enabled */ > + __u64 enable_addr; > + > /* Input: Pointer to string with event name, description and flags */ > __u64 name_args; > > - /* Output: Bitwise index of the event within the status page */ > - __u32 status_bit; > - > /* Output: Index of the event to use when writing data */ > __u32 write_index; > } __attribute__((__packed__)); May want to tab the above too, but as each field is commented, it's not as big of an issue. > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index d7043043f59c..b61a1bfbfc22 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -791,9 +791,10 @@ config USER_EVENTS > can be used like an existing kernel trace event. User trace > events are generated by writing to a tracefs file. User > processes can determine if their tracing events should be > - generated by memory mapping a tracefs file and checking for > - an associated byte being non-zero. > + generated by registering a value and bit with the kernel > + that reflects when it is enabled or not. > > + See Documentation/trace/user_events.rst. > If in doubt, say N. > > config HIST_TRIGGERS > diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c > index 070551480747..553a82ee7aeb 100644 > --- a/kernel/trace/trace_events_user.c > +++ b/kernel/trace/trace_events_user.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include "trace.h" > #include "trace_dynevent.h" > @@ -29,34 +30,11 @@ > #define FIELD_DEPTH_NAME 1 > #define FIELD_DEPTH_SIZE 2 > > -/* > - * Limits how many trace_event calls user processes can create: > - * Must be a power of two of PAGE_SIZE. > - */ > -#define MAX_PAGE_ORDER 0 > -#define MAX_PAGES (1 << MAX_PAGE_ORDER) > -#define MAX_BYTES (MAX_PAGES * PAGE_SIZE) > -#define MAX_EVENTS (MAX_BYTES * 8) > - > /* Limit how long of an event name plus args within the subsystem. */ > #define MAX_EVENT_DESC 512 > #define EVENT_NAME(user_event) ((user_event)->tracepoint.name) > #define MAX_FIELD_ARRAY_SIZE 1024 > > -/* > - * The MAP_STATUS_* macros are used for taking a index and determining the > - * appropriate byte and the bit in the byte to set/reset for an event. > - * > - * The lower 3 bits of the index decide which bit to set. > - * The remaining upper bits of the index decide which byte to use for the bit. > - * > - * This is used when an event has a probe attached/removed to reflect live > - * status of the event wanting tracing or not to user-programs via shared > - * memory maps. > - */ > -#define MAP_STATUS_BYTE(index) ((index) >> 3) > -#define MAP_STATUS_MASK(index) BIT((index) & 7) > - > /* > * Internal bits (kernel side only) to keep track of connected probes: > * These are used when status is requested in text form about an event. These > @@ -70,20 +48,14 @@ > #define EVENT_STATUS_OTHER BIT(7) > > /* > - * Stores the pages, tables, and locks for a group of events. > - * Each logical grouping of events has its own group, with a > - * matching page for status checks within user programs. This > - * allows for isolation of events to user programs by various > - * means. > + * Stores the system name, tables, and locks for a group of events. This > + * allows isolation for events by various means. > */ > struct user_event_group { > - struct page *pages; > - char *register_page_data; > char *system_name; > struct hlist_node node; > struct mutex reg_mutex; > DECLARE_HASHTABLE(register_table, 8); > - DECLARE_BITMAP(page_bitmap, MAX_EVENTS); > }; > > /* Group for init_user_ns mapping, top-most group */ > @@ -106,12 +78,34 @@ struct user_event { > struct list_head fields; > struct list_head validators; > refcount_t refcnt; > - int index; > - int flags; > int min_size; > char status; > }; But these should be tabbed out. > > +/* > + * Stores per-mm/event properties that enable an address to be > + * updated properly for each task. As tasks are forked, we use > + * these to track enablement sites that are tied to an event. > + */ > +struct user_event_enabler { > + struct list_head link; > + struct user_event *event; > + unsigned long addr; > + > + /* Track enable bit, flags, etc. Aligned for bitops. */ > + unsigned int values; > +}; > + And the above. > +/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */ > +#define ENABLE_VAL_BIT_MASK 0x3F > + This is something that can be added later as a clean up, but if there's a real issue found with these patches, then make the next version have the updates. If you do another version, update the tabs of existing structures in a separate patch from any content change. -- Steve