Re: [patch 4/6] perf/core: Split out ringbuffer allocation

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Aug 07 2025 at 16:38, Lorenzo Stoakes wrote:
> On Wed, Aug 06, 2025 at 10:12:58PM +0200, Thomas Gleixner wrote:
> THe second is that you don't seem to be doing:
>
> WARN_ON(!rb && event->rb);
> if (vma->vm_flags & VM_WRITE)
> 	flags |= RING_BUFFER_WRITABLE;
>
> In the aux code any more. Maybe first irrelevant, but second surely
> is?

Yeah. The first one is kinda silly. The second one I dropped unintentionally.

> DIFFERENCES:
>
> If we get to the rb_alloc_aux() bit, we're missing the:
>
> WARN_ON(!rb && event->rb);
> if (vma->vm_flags & VM_WRITE)
> 	flags |= RING_BUFFER_WRITABLE;
>
> Bit for aux case.
>
> Otherwise, it seems to be equivalent.

Thanks for taking the time to go through this.

>> -		if (rb_has_aux(rb)) {
>> -			atomic_inc(&rb->aux_mmap_count);
>> -			ret = 0;
>> -			goto unlock;
>> +		if (event->rb) {
>> +			ret = -EINVAL;
>
> Shouldn't this be if (!event->rb) ?
>
>> +		} else {
>
> Because here you're dereffing event->rb in branch where !event->rb?

Yes. I obviously failed to tested this particular patch alone and that's
fixed up in the next which moves the RB allocation out, so it did not
blow up in my face when I tested the whole pile.

Thanks for spotting!

       tglx