From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Andrew Morton <akpm@linux-foundation.org>,
Beau Belgrave <beaub@linux.microsoft.com>
Subject: [for-next][PATCH 15/25] tracing/user_events: Fixup enable faults asyncly
Date: Wed, 29 Mar 2023 15:45:31 -0400 [thread overview]
Message-ID: <20230329194552.069419879@goodmis.org> (raw)
In-Reply-To: 20230329194516.146147554@goodmis.org
From: Beau Belgrave <beaub@linux.microsoft.com>
When events are enabled within the various tracing facilities, such as
ftrace/perf, the event_mutex is held. As events are enabled pages are
accessed. We do not want page faults to occur under this lock. Instead
queue the fault to a workqueue to be handled in a process context safe
way without the lock.
The enable address is marked faulting while the async fault-in occurs.
This ensures that we don't attempt to fault-in more than is necessary.
Once the page has been faulted in, an address write is re-attempted.
If the page couldn't fault-in, then we wait until the next time the
event is enabled to prevent any potential infinite loops.
Link: https://lkml.kernel.org/r/20230328235219.203-5-beaub@linux.microsoft.com
Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_user.c | 120 +++++++++++++++++++++++++++++--
1 file changed, 114 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index 553a82ee7aeb..86bda1660536 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -99,9 +99,23 @@ struct user_event_enabler {
/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
#define ENABLE_VAL_BIT_MASK 0x3F
+/* Bit 6 is for faulting status of enablement */
+#define ENABLE_VAL_FAULTING_BIT 6
+
/* Only duplicate the bit value */
#define ENABLE_VAL_DUP_MASK ENABLE_VAL_BIT_MASK
+#define ENABLE_BITOPS(e) ((unsigned long *)&(e)->values)
+
+/* Used for asynchronous faulting in of pages */
+struct user_event_enabler_fault {
+ struct work_struct work;
+ struct user_event_mm *mm;
+ struct user_event_enabler *enabler;
+};
+
+static struct kmem_cache *fault_cache;
+
/* Global list of memory descriptors using user_events */
static LIST_HEAD(user_event_mms);
static DEFINE_SPINLOCK(user_event_mms_lock);
@@ -263,7 +277,85 @@ static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr)
}
static int user_event_enabler_write(struct user_event_mm *mm,
- struct user_event_enabler *enabler)
+ struct user_event_enabler *enabler,
+ bool fixup_fault);
+
+static void user_event_enabler_fault_fixup(struct work_struct *work)
+{
+ struct user_event_enabler_fault *fault = container_of(
+ work, struct user_event_enabler_fault, work);
+ struct user_event_enabler *enabler = fault->enabler;
+ struct user_event_mm *mm = fault->mm;
+ unsigned long uaddr = enabler->addr;
+ int ret;
+
+ ret = user_event_mm_fault_in(mm, uaddr);
+
+ if (ret && ret != -ENOENT) {
+ struct user_event *user = enabler->event;
+
+ pr_warn("user_events: Fault for mm: 0x%pK @ 0x%llx event: %s\n",
+ mm->mm, (unsigned long long)uaddr, EVENT_NAME(user));
+ }
+
+ /* Prevent state changes from racing */
+ mutex_lock(&event_mutex);
+
+ /*
+ * If we managed to get the page, re-issue the write. We do not
+ * want to get into a possible infinite loop, which is why we only
+ * attempt again directly if the page came in. If we couldn't get
+ * the page here, then we will try again the next time the event is
+ * enabled/disabled.
+ */
+ clear_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler));
+
+ if (!ret) {
+ mmap_read_lock(mm->mm);
+ user_event_enabler_write(mm, enabler, true);
+ mmap_read_unlock(mm->mm);
+ }
+
+ mutex_unlock(&event_mutex);
+
+ /* In all cases we no longer need the mm or fault */
+ user_event_mm_put(mm);
+ kmem_cache_free(fault_cache, fault);
+}
+
+static bool user_event_enabler_queue_fault(struct user_event_mm *mm,
+ struct user_event_enabler *enabler)
+{
+ struct user_event_enabler_fault *fault;
+
+ fault = kmem_cache_zalloc(fault_cache, GFP_NOWAIT | __GFP_NOWARN);
+
+ if (!fault)
+ return false;
+
+ INIT_WORK(&fault->work, user_event_enabler_fault_fixup);
+ fault->mm = user_event_mm_get(mm);
+ fault->enabler = enabler;
+
+ /* Don't try to queue in again while we have a pending fault */
+ set_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler));
+
+ if (!schedule_work(&fault->work)) {
+ /* Allow another attempt later */
+ clear_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler));
+
+ user_event_mm_put(mm);
+ kmem_cache_free(fault_cache, fault);
+
+ return false;
+ }
+
+ return true;
+}
+
+static int user_event_enabler_write(struct user_event_mm *mm,
+ struct user_event_enabler *enabler,
+ bool fixup_fault)
{
unsigned long uaddr = enabler->addr;
unsigned long *ptr;
@@ -278,11 +370,19 @@ static int user_event_enabler_write(struct user_event_mm *mm,
if (refcount_read(&mm->tasks) == 0)
return -ENOENT;
+ if (unlikely(test_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler))))
+ return -EBUSY;
+
ret = pin_user_pages_remote(mm->mm, uaddr, 1, FOLL_WRITE | FOLL_NOFAULT,
&page, NULL, NULL);
- if (ret <= 0) {
- pr_warn("user_events: Enable write failed\n");
+ if (unlikely(ret <= 0)) {
+ if (!fixup_fault)
+ return -EFAULT;
+
+ if (!user_event_enabler_queue_fault(mm, enabler))
+ pr_warn("user_events: Unable to queue fault handler\n");
+
return -EFAULT;
}
@@ -314,7 +414,7 @@ static void user_event_enabler_update(struct user_event *user)
list_for_each_entry_rcu(enabler, &mm->enablers, link)
if (enabler->event == user)
- user_event_enabler_write(mm, enabler);
+ user_event_enabler_write(mm, enabler, true);
rcu_read_unlock();
mmap_read_unlock(mm->mm);
@@ -562,7 +662,7 @@ static struct user_event_enabler
/* Attempt to reflect the current state within the process */
mmap_read_lock(user_mm->mm);
- *write_result = user_event_enabler_write(user_mm, enabler);
+ *write_result = user_event_enabler_write(user_mm, enabler, false);
mmap_read_unlock(user_mm->mm);
/*
@@ -2201,16 +2301,24 @@ static int __init trace_events_user_init(void)
{
int ret;
+ fault_cache = KMEM_CACHE(user_event_enabler_fault, 0);
+
+ if (!fault_cache)
+ return -ENOMEM;
+
init_group = user_event_group_create(&init_user_ns);
- if (!init_group)
+ if (!init_group) {
+ kmem_cache_destroy(fault_cache);
return -ENOMEM;
+ }
ret = create_user_tracefs();
if (ret) {
pr_warn("user_events could not register with tracefs\n");
user_event_group_destroy(init_group);
+ kmem_cache_destroy(fault_cache);
init_group = NULL;
return ret;
}
--
2.39.1
next prev parent reply other threads:[~2023-03-29 19:46 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-29 19:45 [for-next][PATCH 00/25] tracing: Updates for 6.4 Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 01/25] fprobe: Pass entry_data to handlers Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 02/25] lib/test_fprobe: Add private entry_data testcases Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 03/25] fprobe: Add nr_maxactive to specify rethook_node pool size Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 04/25] lib/test_fprobe: Add a test case for nr_maxactive Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 05/25] fprobe: Skip exit_handler if entry_handler returns !0 Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 06/25] lib/test_fprobe: Add a testcase for skipping exit_handler Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 07/25] docs: tracing: Update fprobe documentation Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 08/25] selftests: use canonical ftrace path Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 09/25] leaking_addresses: also skip " Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 10/25] tools/kvm_stat: use " Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 11/25] tracing: Add "fields" option to show raw trace event fields Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 12/25] tracing/user_events: Split header into uapi and kernel Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 13/25] tracing/user_events: Track fork/exec/exit for mm lifetime Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 14/25] tracing/user_events: Use remote writes for event enablement Steven Rostedt
2023-03-29 19:45 ` Steven Rostedt [this message]
2023-03-29 19:45 ` [for-next][PATCH 16/25] tracing/user_events: Add ioctl for disabling addresses Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 17/25] tracing/user_events: Update self-tests to write ABI Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 18/25] tracing/user_events: Add ABI self-test Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 19/25] tracing/user_events: Use write ABI in example Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 20/25] tracing/user_events: Update documentation for ABI Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 21/25] tracing/user_events: Charge event allocs to cgroups Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 22/25] tracing/user_events: Limit global user_event count Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 23/25] tracing/user_events: Align structs with tabs for readability Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 24/25] tracing/user_events: Use print_format_fields() for trace output Steven Rostedt
2023-03-29 19:45 ` [for-next][PATCH 25/25] tracing: Unbreak user events Steven Rostedt
2023-03-29 20:29 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230329194552.069419879@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=beaub@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.