Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] perf: Fix the mlock accounting, again
To:     Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>
Cc:     Ingo Molnar <mingo@redhat.com>, linux-kernel@vger.kernel.org
References: <20191115160818.6480-1-alexander.shishkin@linux.intel.com>
From:   Thomas Richter <tmricht@linux.ibm.com>
Organization: IBM
Date:   Mon, 18 Nov 2019 15:16:22 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20191115160818.6480-1-alexander.shishkin@linux.intel.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Message-Id: <341c1186-888f-3cd4-069b-4ba252f23958@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/15/19 5:08 PM, Alexander Shishkin wrote:
> Commit
> 
>   5e6c3c7b1ec2 ("perf/aux: Fix tracking of auxiliary trace buffer allocation")
> 
> tried to guess the correct combination of arithmetic operations that would
> undo the AUX buffer's mlock accounting, and failed, leaking the bottom part
> when an allocation needs to be changed partially to both user->locked_vm
> and mm->pinned_vm, eventually leaving the user with no locked bonus:
> 
> $ perf record -e intel_pt//u -m1,128 uname
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.061 MB perf.data ]
> $ perf record -e intel_pt//u -m1,128 uname
> Permission error mapping pages.
> Consider increasing /proc/sys/kernel/perf_event_mlock_kb,
> or try again with a smaller value of -m/--mmap_pages.
> (current value: 1,128)
> 
> Fix this by subtracting both locked and pinned counts when AUX buffer is
> unmapped.
> 
> Signefdoff-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  kernel/events/core.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 16d80ad8d6d7..522438887206 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5664,10 +5664,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>  		perf_pmu_output_stop(event);
>  
>  		/* now it's safe to free the pages */
> -		if (!rb->aux_mmap_locked)
> -			atomic_long_sub(rb->aux_nr_pages, &mmap_user->locked_vm);
> -		else
> -			atomic64_sub(rb->aux_mmap_locked, &vma->vm_mm->pinned_vm);
> +		atomic_long_sub(rb->aux_nr_pages - rb->aux_mmap_locked, &mmap_user->locked_vm);
> +		atomic64_sub(rb->aux_mmap_locked, &vma->vm_mm->pinned_vm);
>  
>  		/* this has to be the last one */
>  		rb_free_aux(rb);
> 

I have tested your patch on our test suite which discovered the original issue.
Your patch works nicely, thanks for fixing this.

-- 
Thomas Richter, Dept 3252, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294