From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751697AbcJKFGa (ORCPT ); Tue, 11 Oct 2016 01:06:30 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:33390 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750717AbcJKFG2 (ORCPT ); Tue, 11 Oct 2016 01:06:28 -0400 MIME-Version: 1.0 In-Reply-To: <20161009192610.GB2718@nuc-i3427.alporthouse.com> References: <20160929073411.3154-1-jszhang@marvell.com> <20160929081818.GE28107@nuc-i3427.alporthouse.com> <20161009124242.GA2718@nuc-i3427.alporthouse.com> <20161009192610.GB2718@nuc-i3427.alporthouse.com> From: Joel Fernandes Date: Mon, 10 Oct 2016 22:06:25 -0700 Message-ID: Subject: Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency To: Chris Wilson Cc: Joel Fernandes , Jisheng Zhang , npiggin@kernel.dk, Linux Kernel Mailing List , linux-mm@kvack.org, rientjes@google.com, Andrew Morton , mgorman@techsingularity.net, iamjoonsoo.kim@lge.com, Linux ARM Kernel List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Oct 9, 2016 at 12:26 PM, Chris Wilson wrote: > On Sun, Oct 09, 2016 at 12:00:31PM -0700, Joel Fernandes wrote: >> 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. > > The downside of using a mutex here though, is that we may be called > from contexts that cannot sleep (alloc_vmap_area), or reschedule for > that matter! If we change the notion of purged, we can forgo the mutex > in favour of spinning on the direct reclaim path. That just leaves the > complication of whether to use cond_resched_lock() or a lock around > the individual __free_vmap_area(). Good point. I agree with you. I think we still need to know if purging is in progress to preserve previous trylock behavior. How about something like the following diff? (diff is untested). This drops the purge lock and uses a ref count to indicate if purging is in progress, so that callers who don't want to purge if purging is already in progress can be kept happy. Also I am reducing vmap_lazy_nr as we go, and, not all at once, so that we don't reduce the counter too soon as we're not holding purge lock anymore. Lastly, I added the cond_resched as you suggested. diff --git a/mm/vmalloc.c b/mm/vmalloc.c index f2481cb..5616ca4 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -626,7 +626,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 atomic_t purging; struct llist_node *valist; struct vmap_area *va; struct vmap_area *n_va; @@ -638,10 +638,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 (atomic_cmpxchg(&purging, 0, 1)) return; } else - spin_lock(&purge_lock); + atomic_inc(&purging); if (sync) purge_fragmented_blocks_allcpus(); @@ -655,9 +655,6 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, nr += (va->va_end - va->va_start) >> PAGE_SHIFT; } - if (nr) - atomic_sub(nr, &vmap_lazy_nr); - if (nr || force_flush) flush_tlb_kernel_range(*start, *end); @@ -665,9 +662,11 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, spin_lock(&vmap_area_lock); llist_for_each_entry_safe(va, n_va, valist, purge_list) __free_vmap_area(va); + atomic_sub(1, &vmap_lazy_nr); + cond_resched_lock(&vmap_area_lock); spin_unlock(&vmap_area_lock); } - spin_unlock(&purge_lock); + atomic_dec(&purging); } From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f70.google.com (mail-lf0-f70.google.com [209.85.215.70]) by kanga.kvack.org (Postfix) with ESMTP id 931B66B0038 for ; Tue, 11 Oct 2016 01:06:28 -0400 (EDT) Received: by mail-lf0-f70.google.com with SMTP id n3so6224604lfn.5 for ; Mon, 10 Oct 2016 22:06:28 -0700 (PDT) Received: from mail-lf0-x242.google.com (mail-lf0-x242.google.com. [2a00:1450:4010:c07::242]) by mx.google.com with ESMTPS id r78si762101lff.309.2016.10.10.22.06.26 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Oct 2016 22:06:27 -0700 (PDT) Received: by mail-lf0-x242.google.com with SMTP id x79so1609848lff.2 for ; Mon, 10 Oct 2016 22:06:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20161009192610.GB2718@nuc-i3427.alporthouse.com> References: <20160929073411.3154-1-jszhang@marvell.com> <20160929081818.GE28107@nuc-i3427.alporthouse.com> <20161009124242.GA2718@nuc-i3427.alporthouse.com> <20161009192610.GB2718@nuc-i3427.alporthouse.com> From: Joel Fernandes Date: Mon, 10 Oct 2016 22:06:25 -0700 Message-ID: Subject: Re: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency Content-Type: text/plain; charset=UTF-8 Sender: owner-linux-mm@kvack.org List-ID: To: Chris Wilson Cc: Joel Fernandes , Jisheng Zhang , npiggin@kernel.dk, Linux Kernel Mailing List , linux-mm@kvack.org, rientjes@google.com, Andrew Morton , mgorman@techsingularity.net, iamjoonsoo.kim@lge.com, Linux ARM Kernel List On Sun, Oct 9, 2016 at 12:26 PM, Chris Wilson wrote: > On Sun, Oct 09, 2016 at 12:00:31PM -0700, Joel Fernandes wrote: >> 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. > > The downside of using a mutex here though, is that we may be called > from contexts that cannot sleep (alloc_vmap_area), or reschedule for > that matter! If we change the notion of purged, we can forgo the mutex > in favour of spinning on the direct reclaim path. That just leaves the > complication of whether to use cond_resched_lock() or a lock around > the individual __free_vmap_area(). Good point. I agree with you. I think we still need to know if purging is in progress to preserve previous trylock behavior. How about something like the following diff? (diff is untested). This drops the purge lock and uses a ref count to indicate if purging is in progress, so that callers who don't want to purge if purging is already in progress can be kept happy. Also I am reducing vmap_lazy_nr as we go, and, not all at once, so that we don't reduce the counter too soon as we're not holding purge lock anymore. Lastly, I added the cond_resched as you suggested. diff --git a/mm/vmalloc.c b/mm/vmalloc.c index f2481cb..5616ca4 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -626,7 +626,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 atomic_t purging; struct llist_node *valist; struct vmap_area *va; struct vmap_area *n_va; @@ -638,10 +638,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 (atomic_cmpxchg(&purging, 0, 1)) return; } else - spin_lock(&purge_lock); + atomic_inc(&purging); if (sync) purge_fragmented_blocks_allcpus(); @@ -655,9 +655,6 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, nr += (va->va_end - va->va_start) >> PAGE_SHIFT; } - if (nr) - atomic_sub(nr, &vmap_lazy_nr); - if (nr || force_flush) flush_tlb_kernel_range(*start, *end); @@ -665,9 +662,11 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, spin_lock(&vmap_area_lock); llist_for_each_entry_safe(va, n_va, valist, purge_list) __free_vmap_area(va); + atomic_sub(1, &vmap_lazy_nr); + cond_resched_lock(&vmap_area_lock); spin_unlock(&vmap_area_lock); } - spin_unlock(&purge_lock); + atomic_dec(&purging); } -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: agnel.joel@gmail.com (Joel Fernandes) Date: Mon, 10 Oct 2016 22:06:25 -0700 Subject: [PATCH] mm/vmalloc: reduce the number of lazy_max_pages to reduce latency In-Reply-To: <20161009192610.GB2718@nuc-i3427.alporthouse.com> References: <20160929073411.3154-1-jszhang@marvell.com> <20160929081818.GE28107@nuc-i3427.alporthouse.com> <20161009124242.GA2718@nuc-i3427.alporthouse.com> <20161009192610.GB2718@nuc-i3427.alporthouse.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Sun, Oct 9, 2016 at 12:26 PM, Chris Wilson wrote: > On Sun, Oct 09, 2016 at 12:00:31PM -0700, Joel Fernandes wrote: >> 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. > > The downside of using a mutex here though, is that we may be called > from contexts that cannot sleep (alloc_vmap_area), or reschedule for > that matter! If we change the notion of purged, we can forgo the mutex > in favour of spinning on the direct reclaim path. That just leaves the > complication of whether to use cond_resched_lock() or a lock around > the individual __free_vmap_area(). Good point. I agree with you. I think we still need to know if purging is in progress to preserve previous trylock behavior. How about something like the following diff? (diff is untested). This drops the purge lock and uses a ref count to indicate if purging is in progress, so that callers who don't want to purge if purging is already in progress can be kept happy. Also I am reducing vmap_lazy_nr as we go, and, not all at once, so that we don't reduce the counter too soon as we're not holding purge lock anymore. Lastly, I added the cond_resched as you suggested. diff --git a/mm/vmalloc.c b/mm/vmalloc.c index f2481cb..5616ca4 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -626,7 +626,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 atomic_t purging; struct llist_node *valist; struct vmap_area *va; struct vmap_area *n_va; @@ -638,10 +638,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 (atomic_cmpxchg(&purging, 0, 1)) return; } else - spin_lock(&purge_lock); + atomic_inc(&purging); if (sync) purge_fragmented_blocks_allcpus(); @@ -655,9 +655,6 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, nr += (va->va_end - va->va_start) >> PAGE_SHIFT; } - if (nr) - atomic_sub(nr, &vmap_lazy_nr); - if (nr || force_flush) flush_tlb_kernel_range(*start, *end); @@ -665,9 +662,11 @@ static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, spin_lock(&vmap_area_lock); llist_for_each_entry_safe(va, n_va, valist, purge_list) __free_vmap_area(va); + atomic_sub(1, &vmap_lazy_nr); + cond_resched_lock(&vmap_area_lock); spin_unlock(&vmap_area_lock); } - spin_unlock(&purge_lock); + atomic_dec(&purging); }