linux-kernel - Re: [PATCH v6 02/24] ring-buffer: Introduce ring-buffer remotes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250908191343.70aec65c@gandalf.local.home>
Date: Mon, 8 Sep 2025 19:13:43 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Vincent Donnefort <vdonnefort@...gle.com>
Cc: mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
 linux-trace-kernel@...r.kernel.org, maz@...nel.org, oliver.upton@...ux.dev,
 joey.gouly@....com, suzuki.poulose@....com, yuzenghui@...wei.com,
 kvmarm@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 jstultz@...gle.com, qperret@...gle.com, will@...nel.org,
 aneesh.kumar@...nel.org, kernel-team@...roid.com,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 02/24] ring-buffer: Introduce ring-buffer remotes

On Thu, 21 Aug 2025 09:13:50 +0100
Vincent Donnefort <vdonnefort@...gle.com> wrote:

> +int ring_buffer_poll_remote(struct trace_buffer *buffer, int cpu);
> +
> +struct trace_buffer *
> +__ring_buffer_alloc_remote(struct ring_buffer_remote *remote,
> +			   struct lock_class_key *key);
> +
> +#define ring_buffer_remote(remote)				\
> +({								\
> +	static struct lock_class_key __key;			\
> +	__ring_buffer_alloc_remote(remote, &__key);		\

If this allocates, then it should have "alloc" in its name too. Why isn't this:

#define ring_buffer_alloc_remote(remote)			\

?

> +})
>  #endif /* _LINUX_RING_BUFFER_H */
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 064005c9347e..8044cb23ef88 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -523,6 +523,8 @@ struct ring_buffer_per_cpu {
>  	struct trace_buffer_meta	*meta_page;
>  	struct ring_buffer_cpu_meta	*ring_meta;
>  
> +	struct ring_buffer_remote	*remote;
> +
>  	/* ring buffer pages to update, > 0 to add, < 0 to remove */
>  	long				nr_pages_to_update;
>  	struct list_head		new_pages; /* new pages to add */
> @@ -545,6 +547,8 @@ struct trace_buffer {
>  
>  	struct ring_buffer_per_cpu	**buffers;
>  
> +	struct ring_buffer_remote	*remote;
> +
>  	struct hlist_node		node;
>  	u64				(*clock)(void);
>  
> @@ -2296,6 +2300,40 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
>  	return -ENOMEM;
>  }
> 


> +int ring_buffer_poll_remote(struct trace_buffer *buffer, int cpu)
> +{
> +	struct ring_buffer_per_cpu *cpu_buffer;
> +	unsigned long flags;
> +
> +	if (cpu != RING_BUFFER_ALL_CPUS) {
> +		if (!cpumask_test_cpu(cpu, buffer->cpumask))
> +			return -EINVAL;
> +
> +		cpu_buffer = buffer->buffers[cpu];
> +
> +		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);

The above can be replaced by:

		guard(raw_spinlock)(&cpu_buffer->reader_lock);

> +		if (rb_read_remote_meta_page(cpu_buffer))
> +			rb_wakeups(buffer, cpu_buffer);
> +		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);

Then we don't need the unlock.

> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * Make sure all the ring buffers are up to date before we start reading
> +	 * them.
> +	 */
> +	for_each_buffer_cpu(buffer, cpu) {
> +		cpu_buffer = buffer->buffers[cpu];
> +
> +		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);

Ditto.

> +		rb_read_remote_meta_page(buffer->buffers[cpu]);
> +		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> +	}
> +
> +	for_each_buffer_cpu(buffer, cpu) {
> +		cpu_buffer = buffer->buffers[cpu];
> +
> +		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);

Ditto.

-- Steve


> +		if (rb_num_of_entries(cpu_buffer))
> +			rb_wakeups(buffer, buffer->buffers[cpu]);
> +		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> +	}
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_RING_BUFFER_ALLOW_SWAP
>  /**
>   * ring_buffer_swap_cpu - swap a CPU buffer between two ring buffers
> @@ -6600,6 +6801,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	unsigned int commit;
>  	unsigned int read;
>  	u64 save_timestamp;
> +	bool force_memcpy;
>  
>  	if (!cpumask_test_cpu(cpu, buffer->cpumask))
>  		return -1;
> @@ -6637,6 +6839,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	/* Check if any events were dropped */
>  	missed_events = cpu_buffer->lost_events;
>  
> +	force_memcpy = cpu_buffer->mapped || cpu_buffer->remote;
> +
>  	/*
>  	 * If this page has been partially read or
>  	 * if len is not big enough to read the rest of the page or
> @@ -6646,7 +6850,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	 */
>  	if (read || (len < (commit - read)) ||
>  	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
> -	    cpu_buffer->mapped) {
> +	    force_memcpy) {
>  		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
>  		unsigned int rpos = read;
>  		unsigned int pos = 0;
> @@ -7222,7 +7426,7 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
>  	unsigned long flags, *subbuf_ids;
>  	int err;
>  
> -	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> +	if (!cpumask_test_cpu(cpu, buffer->cpumask) || buffer->remote)
>  		return -EINVAL;
>  
>  	cpu_buffer = buffer->buffers[cpu];