Thread (12 messages) 12 messages, 2 authors, 2024-02-12

Re: [PATCH v16 4/6] tracing: Allow user-space mapping of the ring-buffer

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-02-11 22:31:58
Also in: lkml

On Fri,  9 Feb 2024 16:34:46 +0000
Vincent Donnefort [off-list ref] wrote:
+static void tracing_buffers_mmap_close(struct vm_area_struct *vma)
+{
+	struct ftrace_buffer_info *info = vma->vm_file->private_data;
+	struct trace_iterator *iter = &info->iter;
+	struct trace_array __maybe_unused *tr = iter->tr;
+
+	ring_buffer_unmap(iter->array_buffer->buffer, iter->cpu_file);
+
+#ifdef CONFIG_TRACER_MAX_TRACE
+	spin_lock(&tr->snapshot_trigger_lock);
+	if (!WARN_ON(!tr->mapped))
+		tr->mapped--;
+	spin_unlock(&tr->snapshot_trigger_lock);
+#endif
You can add a section with the #ifdef *MAX_TRACE and use static inline
for these (or use an existing section):

#ifdef CONFIG_TRACER_MAX_TRACE
static void put_snapshot_map(struct trace_array *tr)
{
	spin_lock(&tr->snapshot_trigger_lock);
	if (!WARN_ON(!tr->mapped))
		tr->mapped--;
	spin_unlock(&tr->snapshot_trigger_lock);	
}
[..]
#else
static inline void put_snapshot_map(struct trace_array *tr) { }
[..]
#endif
+}
+
+static void tracing_buffers_mmap_open(struct vm_area_struct *vma)
+{
+	struct ftrace_buffer_info *info = vma->vm_file->private_data;
+	struct trace_iterator *iter = &info->iter;
+
+	WARN_ON(ring_buffer_map(iter->array_buffer->buffer, iter->cpu_file));
+}
+
+static const struct vm_operations_struct tracing_buffers_vmops = {
+	.open		= tracing_buffers_mmap_open,
+	.close		= tracing_buffers_mmap_close,
+	.fault		= tracing_buffers_mmap_fault,
+};
+
+static int tracing_buffers_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct ftrace_buffer_info *info = filp->private_data;
+	struct trace_iterator *iter = &info->iter;
+	struct trace_array __maybe_unused *tr = iter->tr;
+	int ret = 0;
+
+	if (vma->vm_flags & VM_WRITE || vma->vm_flags & VM_EXEC)
+		return -EPERM;
+
+	vm_flags_mod(vma, VM_DONTCOPY | VM_DONTDUMP, VM_MAYWRITE);
+	vma->vm_ops = &tracing_buffers_vmops;
+
+#ifdef CONFIG_TRACER_MAX_TRACE
+	/*
+	 * We hold mmap_lock here. lockdep would be unhappy if we would now take
+	 * trace_types_lock. Instead use the specific snapshot_trigger_lock.
+	 */
+	spin_lock(&tr->snapshot_trigger_lock);
+	if (tr->snapshot || tr->mapped == UINT_MAX) {
+		spin_unlock(&tr->snapshot_trigger_lock);
+		return -EBUSY;
+	}
+	tr->mapped++;
+	spin_unlock(&tr->snapshot_trigger_lock);
+
+	/* Wait for update_max_tr() to observe iter->tr->mapped */
+	if (tr->mapped == 1)
+		synchronize_rcu();
+#endif
The above could be:

static int get_snapshot_map(struct trace_array *tr)
{
	/*
	 * We hold mmap_lock here. lockdep would be unhappy if we would now take
	 * trace_types_lock. Instead use the specific snapshot_trigger_lock.
	 */
	spin_lock(&tr->snapshot_trigger_lock);
	if (tr->snapshot || tr->mapped == UINT_MAX) {
		spin_unlock(&tr->snapshot_trigger_lock);
		return -EBUSY;
	}
	tr->mapped++;
	spin_unlock(&tr->snapshot_trigger_lock);

	/* Wait for update_max_tr() to observe iter->tr->mapped */
	if (tr->mapped == 1)
		synchronize_rcu();

	return 0;
}
#else
static inline test_snapshot_map(struct trace_array *tr)
{
	return 0;
}
[..]

Then here just have:

	ret = test_snapshot_map(tr);
	if (ret < 0)
		return ret;

+	ret = ring_buffer_map(iter->array_buffer->buffer, iter->cpu_file);
+#ifdef CONFIG_TRACER_MAX_TRACE
+	if (ret) {
+		spin_lock(&tr->snapshot_trigger_lock);
+		iter->tr->mapped--;
+		spin_unlock(&tr->snapshot_trigger_lock);
+	}
+#endif
and the above is just:

	if (ret)
		put_snapshot_map(tr);

-- Steve
quoted hunk ↗ jump to hunk
+	return ret;
+}
+
 static const struct file_operations tracing_buffers_fops = {
 	.open		= tracing_buffers_open,
 	.read		= tracing_buffers_read,
@@ -8681,6 +8794,7 @@ static const struct file_operations tracing_buffers_fops = {
 	.splice_read	= tracing_buffers_splice_read,
 	.unlocked_ioctl = tracing_buffers_ioctl,
 	.llseek		= no_llseek,
+	.mmap		= tracing_buffers_mmap,
 };
 
 static ssize_t
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bd312e9afe25..8a96e7a89e6b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -336,6 +336,7 @@ struct trace_array {
 	bool			allocated_snapshot;
 	spinlock_t		snapshot_trigger_lock;
 	unsigned int		snapshot;
+	unsigned int		mapped;
 	unsigned long		max_latency;
 #ifdef CONFIG_FSNOTIFY
 	struct dentry		*d_max_latency;
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help