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


Groups > linux.kernel > #1497998

Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency

From Joel Fernandes <joel.opensrc@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency
Date 2016-10-09 21:10 +0200
Message-ID <sqrDX-89N-1@gated-at.bofh.it> (permalink)
References <smE6K-10y-13@gated-at.bofh.it> <smEJr-1zc-1@gated-at.bofh.it> <sqdhD-7qZ-1@gated-at.bofh.it> <sqlId-4dC-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Oct 9, 2016 at 5:42 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
[..]
>> > My understanding is that
>> >
>> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> > index 91f44e78c516..3f7c6d6969ac 100644
>> > --- a/mm/vmalloc.c
>> > +++ b/mm/vmalloc.c
>> > @@ -626,7 +626,6 @@ 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);
>> >         struct llist_node *valist;
>> >         struct vmap_area *va;
>> >         struct vmap_area *n_va;
>> > @@ -637,12 +636,6 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
>> >          * should not expect such behaviour. This just simplifies locking for
>> >          * the case that isn't actually used at the moment anyway.
>> >          */
>> > -       if (!sync && !force_flush) {
>> > -               if (!spin_trylock(&purge_lock))
>> > -                       return;
>> > -       } else
>> > -               spin_lock(&purge_lock);
>> > -
>> >         if (sync)
>> >                 purge_fragmented_blocks_allcpus();
>> >
>> > @@ -667,7 +660,6 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
>> >                         __free_vmap_area(va);
>> >                 spin_unlock(&vmap_area_lock);
>> >         }
>> > -       spin_unlock(&purge_lock);
>> >  }
>> >
>> [..]
>> > should now be safe. That should significantly reduce the preempt-disabled
>> > section, I think.
>>
>> I believe that the purge_lock is supposed to prevent concurrent purges
>> from happening.
>>
>> For the case where if you have another concurrent overflow happen in
>> alloc_vmap_area() between the spin_unlock and purge :
>>
>> spin_unlock(&vmap_area_lock);
>> if (!purged)
>>    purge_vmap_area_lazy();
>>
>> Then the 2 purges would happen at the same time and could subtract
>> vmap_lazy_nr twice.
>
> That itself is not the problem, as each instance of
> __purge_vmap_area_lazy() operates on its own freelist, and so there will
> be no double accounting.
>
> However, removing the lock removes the serialisation which does mean
> that alloc_vmap_area() will not block on another thread conducting the
> purge, and so it will try to reallocate before that is complete and the
> free area made available. It also means that we are doing the
> atomic_sub(vmap_lazy_nr) too early.
>
> That supports making the outer lock a mutex as you suggested. But I think
> cond_resched_lock() is better for the vmap_area_lock (just because it
> turns out to be an expensive loop and we may want the reschedule).
> -Chris

Ok. So I'll submit a patch with mutex for purge_lock and use
cond_resched_lock for the vmap_area_lock as you suggested. I'll also
drop the lazy_max_pages to 8MB as Andi suggested to reduce the lock
hold time. Let me know if you have any objections.

Thanks,
Joel

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Jisheng Zhang <jszhang@marvell.com> - 2016-09-29 09:40 +0200
  Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Chris Wilson <chris@chris-wilson.co.uk> - 2016-09-29 10:20 +0200
    Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Jisheng Zhang <jszhang@marvell.com> - 2016-09-29 10:40 +0200
      Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Chris Wilson <chris@chris-wilson.co.uk> - 2016-09-29 13:10 +0200
        Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Jisheng Zhang <jszhang@marvell.com> - 2016-09-29 13:30 +0200
    Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Joel Fernandes <agnel.joel@gmail.com> - 2016-10-09 05:50 +0200
      Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Chris Wilson <chris@chris-wilson.co.uk> - 2016-10-09 14:50 +0200
        Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Joel Fernandes <joel.opensrc@gmail.com> - 2016-10-09 21:10 +0200
          Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to  reduce latency Chris Wilson <chris@chris-wilson.co.uk> - 2016-10-09 21:30 +0200
            Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Joel Fernandes <agnel.joel@gmail.com> - 2016-10-11 07:10 +0200
              Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Joel Fernandes <agnel.joel@gmail.com> - 2016-10-11 07:40 +0200

csiph-web