[PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

On my workstation the order 4 BTS buffer allocation fails regularly
after the system has been up for some time due to memory
fragmentation.

BTS is virtual memory, so we can just fall back to vmalloc
instead of failing.

Do this here.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 9dc4199..3cf5b74 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -312,8 +312,11 @@ static int alloc_bts_buffer(int cpu)
 
 	buffer = kzalloc_node(BTS_BUFFER_SIZE, GFP_KERNEL | __GFP_NOWARN, node);
 	if (unlikely(!buffer)) {
-		WARN_ONCE(1, "%s: BTS buffer allocation failure\n", __func__);
-		return -ENOMEM;
+		buffer = vmalloc_node(BTS_BUFFER_SIZE, node);
+		if (!buffer) {
+			WARN_ONCE(1, "%s: BTS buffer allocation failure\n", __func__);
+			return -ENOMEM;
+		}
 	}
 
 	max = BTS_BUFFER_SIZE / BTS_RECORD_SIZE;
@@ -336,7 +339,7 @@ static void release_bts_buffer(int cpu)
 	if (!ds || !x86_pmu.bts)
 		return;
 
-	kfree((void *)(unsigned long)ds->bts_buffer_base);
+	kvfree((void *)(unsigned long)ds->bts_buffer_base);
 	ds->bts_buffer_base = 0;
 }
 
-- 
1.9.3

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

On Tue, Sep 09, 2014 at 05:03:21PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> On my workstation the order 4 BTS buffer allocation fails regularly
> after the system has been up for some time due to memory
> fragmentation.
> 
> BTS is virtual memory, so we can just fall back to vmalloc
> instead of failing.
> 
> Do this here.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_ds.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> index 9dc4199..3cf5b74 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> @@ -312,8 +312,11 @@ static int alloc_bts_buffer(int cpu)
>  
>  	buffer = kzalloc_node(BTS_BUFFER_SIZE, GFP_KERNEL | __GFP_NOWARN, node);
>  	if (unlikely(!buffer)) {
> -		WARN_ONCE(1, "%s: BTS buffer allocation failure\n", __func__);
> -		return -ENOMEM;
> +		buffer = vmalloc_node(BTS_BUFFER_SIZE, node);
> +		if (!buffer) {
> +			WARN_ONCE(1, "%s: BTS buffer allocation failure\n", __func__);
> +			return -ENOMEM;
> +		}
>  	}
>  
>  	max = BTS_BUFFER_SIZE / BTS_RECORD_SIZE;

We did this once, and that blew up big time.

Even now, vmalloc_fault() has a very explicit:

  WARN_ON_ONCE(in_nmi());

So this isn't going to happen until you fix that.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Andi Kleen]

> We did this once, and that blew up big time.
> 
> Even now, vmalloc_fault() has a very explicit:
> 
>   WARN_ON_ONCE(in_nmi());
> 
> So this isn't going to happen until you fix that.

Good point.

We just need to call vmalloc_sync_all at allocation time,
no need to change vmalloc_fault.

-Andi



-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Peter Zijlstra]

On Wed, Sep 10, 2014 at 04:40:12PM +0200, Andi Kleen wrote:
> > We did this once, and that blew up big time.
> > 
> > Even now, vmalloc_fault() has a very explicit:
> > 
> >   WARN_ON_ONCE(in_nmi());
> > 
> > So this isn't going to happen until you fix that.
> 
> Good point.
> 
> We just need to call vmalloc_sync_all at allocation time,
> no need to change vmalloc_fault.

Somehow that wasn't considered adequate, but lemme try and dig out that
thread. It should be somewhere...

https://lkml.org/lkml/2010/7/14/465

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Peter Zijlstra]

On Wed, Sep 10, 2014 at 06:35:56PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 10, 2014 at 04:40:12PM +0200, Andi Kleen wrote:
> > > We did this once, and that blew up big time.
> > > 
> > > Even now, vmalloc_fault() has a very explicit:
> > > 
> > >   WARN_ON_ONCE(in_nmi());
> > > 
> > > So this isn't going to happen until you fix that.
> > 
> > Good point.
> > 
> > We just need to call vmalloc_sync_all at allocation time,
> > no need to change vmalloc_fault.
> 
> Somehow that wasn't considered adequate, but lemme try and dig out that
> thread. It should be somewhere...
> 
> https://lkml.org/lkml/2010/7/14/465

Note that since then we have actually fixed the 'cannot fault from NMI
context' thing, so we should be able to actually take those faults. I
suspect we can simply remove those WARNs from the vmalloc fault path,
but it would need double checking to see if there's no other reasons.

Also:

  https://lkml.org/lkml/2014/4/10/581
  https://patchwork.kernel.org/patch/3048111/

I'm not entirely sure what the final conclusion is, except to note that
it didn't happen.

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Andi Kleen]

> Note that since then we have actually fixed the 'cannot fault from NMI
> context' thing, so we should be able to actually take those faults. I
> suspect we can simply remove those WARNs from the vmalloc fault path,
> but it would need double checking to see if there's no other reasons.

I think we actually need to use vmalloc_sync_all, because the PEBS/DS
microcode may do the first access, and those lose events when
the page is not present.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH] perf, x86: Fall back to vmalloc for BTS buffer allocation

[Peter Zijlstra]

On Wed, Sep 10, 2014 at 11:27:44AM -0700, Andi Kleen wrote:
> > Note that since then we have actually fixed the 'cannot fault from NMI
> > context' thing, so we should be able to actually take those faults. I
> > suspect we can simply remove those WARNs from the vmalloc fault path,
> > but it would need double checking to see if there's no other reasons.
> 
> I think we actually need to use vmalloc_sync_all, because the PEBS/DS
> microcode may do the first access, and those lose events when
> the page is not present.

You'll need to convince Linus: https://lkml.org/lkml/2010/7/14/478