Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1363160 > unrolled thread

[RFC] high preempt off latency in vfree path

Started byJoel Fernandes <joelaf@lab126.com>
First post2016-03-23 02:50 +0100
Last post2016-03-23 20:10 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC] high preempt off latency in vfree path Joel Fernandes <joelaf@lab126.com> - 2016-03-23 02:50 +0100
    Re: [RFC] high preempt off latency in vfree path Andi Kleen <ak@linux.intel.com> - 2016-03-23 03:50 +0100
      Re: [RFC] high preempt off latency in vfree path Joel Fernandes <joelaf@lab126.com> - 2016-03-23 20:10 +0100

#1363160 — [RFC] high preempt off latency in vfree path

FromJoel Fernandes <joelaf@lab126.com>
Date2016-03-23 02:50 +0100
Subject[RFC] high preempt off latency in vfree path
Message-ID<rfG5Q-4yd-9@gated-at.bofh.it>
Hi,

I'm seeing on my system with some real time audio requirements, I'm seeing the preemptirqsoff 
tracer complaining that preempt was off for 17ms in the vfree path. Since we have requirements 
of 8ms scheduling this seems awfully bad.

The tracer output showed __free_vmap_area was about 7300 times. Can we do better here? I have 
proposed 2 potential fixes here, any thoughts on which one's better?

Here's the path that blocks preempt (full latency ftrace output uploaded to 
http://raw.codepile.net/pile/OWNpvKkB.js)

  => preempt_count_sub
  => _raw_spin_unlock
  => __purge_vmap_area_lazy
  => free_vmap_area_noflush
  => remove_vm_area
  => __vunmap
  => vfree
  => n_tty_close
  => tty_ldisc_close.isra.1
  => tty_ldisc_kill
  => tty_ldisc_release
  => tty_release

Here are the approaches:
(1)
One is we reduce the number of lazy_max_pages (right now its around 32MB per core worth of pages).

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index aa3891e..2720f4f 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -564,7 +564,7 @@ static unsigned long lazy_max_pages(void)

         log = fls(num_online_cpus());

-       return log * (32UL * 1024 * 1024 / PAGE_SIZE);
+       return log * (8UL * 1024 * 1024 / PAGE_SIZE);
  }


(2) Second alternative approach I am thinking is to change purge_lock into a mutex and then 
move the vmap_area spinlock around the free_vmap_area call. Thus giving the scheduler a chance 
to put something else on the CPU in between free_vmap_area calls. That would look like:

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index aa3891e..9565d72 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -594,7 +594,7 @@ void set_iounmap_nonlazy(void)
  static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
                                         int sync, int force_flush)
  {
-       static DEFINE_SPINLOCK(purge_lock);
+       static DEFINE_MUTEX(purge_lock);
         LIST_HEAD(valist);
         struct vmap_area *va;
         struct vmap_area *n_va;
@@ -606,10 +606,10 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
          * the case that isn't actually used at the moment anyway.
          */
         if (!sync && !force_flush) {
-               if (!spin_trylock(&purge_lock))
+               if (!mutex_trylock(&purge_lock))
                         return;
         } else
-               spin_lock(&purge_lock);
+               mutex_lock(&purge_lock);

         if (sync)
                 purge_fragmented_blocks_allcpus();
@@ -636,12 +636,13 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
                 flush_tlb_kernel_range(*start, *end);

         if (nr) {
-               spin_lock(&vmap_area_lock);
-               list_for_each_entry_safe(va, n_va, &valist, purge_list)
+               list_for_each_entry_safe(va, n_va, &valist, purge_list) {
+                       spin_lock(&vmap_area_lock);
                         __free_vmap_area(va);
+                       spin_unlock(&vmap_area_lock);
+               }
-               spin_unlock(&vmap_area_lock);

         }
-       spin_unlock(&purge_lock);
+       mutex_unlock(&purge_lock);
  }

  /*

Thanks!
Joel

[toc] | [next] | [standalone]


#1363181

FromAndi Kleen <ak@linux.intel.com>
Date2016-03-23 03:50 +0100
Message-ID<rfH1T-5cz-3@gated-at.bofh.it>
In reply to#1363160
> (1)
> One is we reduce the number of lazy_max_pages (right now its around 32MB per core worth of pages).
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index aa3891e..2720f4f 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -564,7 +564,7 @@ static unsigned long lazy_max_pages(void)
> 
>         log = fls(num_online_cpus());
> 
> -       return log * (32UL * 1024 * 1024 / PAGE_SIZE);
> +       return log * (8UL * 1024 * 1024 / PAGE_SIZE);
>  }

This seems like the right fix to me.  Perhaps even make it somewhat smaller.

Even on larger systems it's probably fine because they have a lot more
cores/threads these days, so it will be still sufficiently large.

-Andi

[toc] | [prev] | [next] | [standalone]


#1363618

FromJoel Fernandes <joelaf@lab126.com>
Date2016-03-23 20:10 +0100
Message-ID<rfWkh-7VP-9@gated-at.bofh.it>
In reply to#1363181
On 03/22/16 19:44, Andi Kleen wrote:
>> (1)
>> One is we reduce the number of lazy_max_pages (right now its around 32MB per core worth of pages).
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index aa3891e..2720f4f 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -564,7 +564,7 @@ static unsigned long lazy_max_pages(void)
>>
>>          log = fls(num_online_cpus());
>>
>> -       return log * (32UL * 1024 * 1024 / PAGE_SIZE);
>> +       return log * (8UL * 1024 * 1024 / PAGE_SIZE);
>>   }
>
> This seems like the right fix to me.  Perhaps even make it somewhat smaller.
>
> Even on larger systems it's probably fine because they have a lot more
> cores/threads these days, so it will be still sufficiently large.
>

Thanks Andi. I'll post a patch then.

Regards,
Joel

> -Andi
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web