if (c) {
    X1;
  } else {
    Y;
    goto l;
  }

  X2;
l:

into:

  if (c) {
    X1;
    X2;
  } else {
    Y;
  }

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |   73 ++++++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6664,6 +6664,41 @@ static int perf_mmap(struct file *file,
 
 	if (vma->vm_pgoff == 0) {
 		nr_pages = (vma_size / PAGE_SIZE) - 1;
+
+		/*
+		 * If we have rb pages ensure they're a power-of-two number, so we
+		 * can do bitmasks instead of modulo.
+		 */
+		if (nr_pages != 0 && !is_power_of_2(nr_pages))
+			return -EINVAL;
+
+		if (vma_size != PAGE_SIZE * (1 + nr_pages))
+			return -EINVAL;
+
+		WARN_ON_ONCE(event->ctx->parent_ctx);
+again:
+		mutex_lock(&event->mmap_mutex);
+		if (event->rb) {
+			if (data_page_nr(event->rb) != nr_pages) {
+				ret = -EINVAL;
+				goto unlock;
+			}
+
+			if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
+				/*
+				 * Raced against perf_mmap_close(); remove the
+				 * event and try again.
+				 */
+				ring_buffer_attach(event, NULL);
+				mutex_unlock(&event->mmap_mutex);
+				goto again;
+			}
+
+			rb = event->rb;
+			goto unlock;
+		}
+
+		user_extra = nr_pages + 1;
 	} else {
 		/*
 		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
@@ -6723,47 +6758,9 @@ static int perf_mmap(struct file *file,
 
 		atomic_set(&rb->aux_mmap_count, 1);
 		user_extra = nr_pages;
-
-		goto accounting;
 	}
 
-	/*
-	 * If we have rb pages ensure they're a power-of-two number, so we
-	 * can do bitmasks instead of modulo.
-	 */
-	if (nr_pages != 0 && !is_power_of_2(nr_pages))
-		return -EINVAL;
-
-	if (vma_size != PAGE_SIZE * (1 + nr_pages))
-		return -EINVAL;
-
-	WARN_ON_ONCE(event->ctx->parent_ctx);
-again:
-	mutex_lock(&event->mmap_mutex);
-	if (event->rb) {
-		if (data_page_nr(event->rb) != nr_pages) {
-			ret = -EINVAL;
-			goto unlock;
-		}
-
-		if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
-			/*
-			 * Raced against perf_mmap_close(); remove the
-			 * event and try again.
-			 */
-			ring_buffer_attach(event, NULL);
-			mutex_unlock(&event->mmap_mutex);
-			goto again;
-		}
-
 		/* We need the rb to map pages. */
-		rb = event->rb;
-		goto unlock;
-	}
-
-	user_extra = nr_pages + 1;
-
-accounting:
 	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
 
 	/*

Hi Peter,

Minor nit below:

> +			if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
> +				/*
> +				 * Raced against perf_mmap_close(); remove the
> +				 * event and try again.
> +				 */
> +				ring_buffer_attach(event, NULL);
> +				mutex_unlock(&event->mmap_mutex);
> +				goto again;
> +			}
> +

here ...

> +			rb = event->rb;
> +			goto unlock;
> +		}
> +
> +		user_extra = nr_pages + 1;
>  	} else {
>  		/*
>  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> @@ -6723,47 +6758,9 @@ static int perf_mmap(struct file *file,
>  
>  		atomic_set(&rb->aux_mmap_count, 1);
>  		user_extra = nr_pages;
> -
> -		goto accounting;
>  	}
>  
> -	/*
> -	 * If we have rb pages ensure they're a power-of-two number, so we
> -	 * can do bitmasks instead of modulo.
> -	 */
> -	if (nr_pages != 0 && !is_power_of_2(nr_pages))
> -		return -EINVAL;
> -
> -	if (vma_size != PAGE_SIZE * (1 + nr_pages))
> -		return -EINVAL;
> -
> -	WARN_ON_ONCE(event->ctx->parent_ctx);
> -again:
> -	mutex_lock(&event->mmap_mutex);
> -	if (event->rb) {
> -		if (data_page_nr(event->rb) != nr_pages) {
> -			ret = -EINVAL;
> -			goto unlock;
> -		}
> -
> -		if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
> -			/*
> -			 * Raced against perf_mmap_close(); remove the
> -			 * event and try again.
> -			 */
> -			ring_buffer_attach(event, NULL);
> -			mutex_unlock(&event->mmap_mutex);
> -			goto again;
> -		}
> -
>  		/* We need the rb to map pages. */

                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This comment should also go up. Keeping it here is misleading.

> -		rb = event->rb;
> -		goto unlock;
> -	}
> -
> -	user_extra = nr_pages + 1;
> -
> -accounting:
>  	user_lock_limit = sysctl_perf_event_mlock >> (PAGE_SHIFT - 10);
>  
>  	/*

Thanks,
Ravi

* Ravi Bangoria <ravi.bangoria@amd.com> wrote:

> Hi Peter,
> 
> Minor nit below:
> 
> > +			if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
> > +				/*
> > +				 * Raced against perf_mmap_close(); remove the
> > +				 * event and try again.
> > +				 */
> > +				ring_buffer_attach(event, NULL);
> > +				mutex_unlock(&event->mmap_mutex);
> > +				goto again;
> > +			}
> > +
> 
> here ...
> 
> > +			rb = event->rb;
> > +			goto unlock;
> > +		}
> > +
> > +		user_extra = nr_pages + 1;
> >  	} else {
> >  		/*
> >  		 * AUX area mapping: if rb->aux_nr_pages != 0, it's already
> > @@ -6723,47 +6758,9 @@ static int perf_mmap(struct file *file,
> >  
> >  		atomic_set(&rb->aux_mmap_count, 1);
> >  		user_extra = nr_pages;
> > -
> > -		goto accounting;
> >  	}
> >  
> > -	/*
> > -	 * If we have rb pages ensure they're a power-of-two number, so we
> > -	 * can do bitmasks instead of modulo.
> > -	 */
> > -	if (nr_pages != 0 && !is_power_of_2(nr_pages))
> > -		return -EINVAL;
> > -
> > -	if (vma_size != PAGE_SIZE * (1 + nr_pages))
> > -		return -EINVAL;
> > -
> > -	WARN_ON_ONCE(event->ctx->parent_ctx);
> > -again:
> > -	mutex_lock(&event->mmap_mutex);
> > -	if (event->rb) {
> > -		if (data_page_nr(event->rb) != nr_pages) {
> > -			ret = -EINVAL;
> > -			goto unlock;
> > -		}
> > -
> > -		if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
> > -			/*
> > -			 * Raced against perf_mmap_close(); remove the
> > -			 * event and try again.
> > -			 */
> > -			ring_buffer_attach(event, NULL);
> > -			mutex_unlock(&event->mmap_mutex);
> > -			goto again;
> > -		}
> > -
> >  		/* We need the rb to map pages. */
> 
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This comment should also go up. Keeping it here is misleading.

Yeah, I did that in the conflict resolution (which conflicted in this 
very area), so the end result in tip:perf/core (or tip:master) should 
be fine as-is, correct?

Thanks,

	Ingo

Hi Ingo,

>>> -
>>>  		/* We need the rb to map pages. */
>>
>>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> This comment should also go up. Keeping it here is misleading.
> 
> Yeah, I did that in the conflict resolution (which conflicted in this 
> very area), so the end result in tip:perf/core (or tip:master) should 
> be fine as-is, correct?

Yes, tip:perf/core looks good.

Thanks,
Ravi

* Ravi Bangoria <ravi.bangoria@amd.com> wrote:

> Hi Ingo,
> 
> >>> -
> >>>  		/* We need the rb to map pages. */
> >>
> >>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >> This comment should also go up. Keeping it here is misleading.
> > 
> > Yeah, I did that in the conflict resolution (which conflicted in this 
> > very area), so the end result in tip:perf/core (or tip:master) should 
> > be fine as-is, correct?
> 
> Yes, tip:perf/core looks good.

Great, thank you for checking!

I also picked up:

  bfd33e88addd perf/core: Fix perf_mmap() failure path

and have added your Reviewed-by tags to the series.

Thanks,

	Ingo