linux-kernel - Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping functions

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: <6apbwt27afeercymrjsulrgyv4c2pj2tzqdllukf4czsbfifqo@6p23uqdjqtu7>
Date: Tue, 23 Apr 2024 12:04:15 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Vincent Donnefort <vdonnefort@...gle.com>, mhiramat@...nel.org,
        linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        mathieu.desnoyers@...icios.com, kernel-team@...roid.com,
        rdunlap@...radead.org, linux-mm@...ck.org
Subject: Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping
 functions

* Steven Rostedt <rostedt@...dmis.org> [240410 13:41]:
> On Sat,  6 Apr 2024 18:36:46 +0100
> Vincent Donnefort <vdonnefort@...gle.com> wrote:
> 
> > +int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> > +		    struct vm_area_struct *vma)
> > +{
> > +	struct ring_buffer_per_cpu *cpu_buffer;
> > +	unsigned long flags, *subbuf_ids;
> > +	int err = 0;
> > +
> > +	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > +		return -EINVAL;
> > +
> > +	cpu_buffer = buffer->buffers[cpu];
> > +
> > +	mutex_lock(&cpu_buffer->mapping_lock);
> > +
> > +	if (cpu_buffer->mapped) {
> > +		err = __rb_map_vma(cpu_buffer, vma);
> > +		if (!err)
> > +			err = __rb_inc_dec_mapped(cpu_buffer, true);
> > +		mutex_unlock(&cpu_buffer->mapping_lock);
> > +		return err;
> > +	}
> > +
> > +	/* prevent another thread from changing buffer/sub-buffer sizes */
> > +	mutex_lock(&buffer->mutex);
> > +
> > +	err = rb_alloc_meta_page(cpu_buffer);
> > +	if (err)
> > +		goto unlock;
> > +
> > +	/* subbuf_ids include the reader while nr_pages does not */
> > +	subbuf_ids = kcalloc(cpu_buffer->nr_pages + 1, sizeof(*subbuf_ids), GFP_KERNEL);
> > +	if (!subbuf_ids) {
> > +		rb_free_meta_page(cpu_buffer);
> > +		err = -ENOMEM;
> > +		goto unlock;
> > +	}
> > +
> > +	atomic_inc(&cpu_buffer->resize_disabled);
> > +
> > +	/*
> > +	 * Lock all readers to block any subbuf swap until the subbuf IDs are
> > +	 * assigned.
> > +	 */
> > +	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> > +	rb_setup_ids_meta_page(cpu_buffer, subbuf_ids);
> > +	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> > +
> > +	err = __rb_map_vma(cpu_buffer, vma);
> > +	if (!err) {
> > +		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
> > +		cpu_buffer->mapped = 1;
> > +		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> > +	} else {
> > +		kfree(cpu_buffer->subbuf_ids);
> > +		cpu_buffer->subbuf_ids = NULL;
> > +		rb_free_meta_page(cpu_buffer);
> > +	}
> > +unlock:
> 
> Nit: For all labels, please add a space before them. Otherwise, diffs will
> show "unlock" as the function and not "ring_buffer_map", making it harder
> to find where the change is.
> 

Isn't the inclusion of a space before labels outdate since 'git diff'
superseds 'diff' and fixed this issue?

Thanks,
Liam