Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754175AbcJOQyM (ORCPT ); Sat, 15 Oct 2016 12:54:12 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:33285 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751529AbcJOQyG (ORCPT ); Sat, 15 Oct 2016 12:54:06 -0400 Date: Sat, 15 Oct 2016 09:54:05 -0700 From: Christoph Hellwig To: Joel Fernandes Cc: linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, Chris Wilson , Jisheng Zhang , John Dias , Andrew Morton , "open list:MEMORY MANAGEMENT" Subject: Re: [PATCH v3] mm: vmalloc: Replace purge_lock spinlock with atomic refcount Message-ID: <20161015165405.GA31568@infradead.org> References: <1476535979-27467-1-git-send-email-joelaf@google.com> <20161015164613.GA26079@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161015164613.GA26079@infradead.org> User-Agent: Mutt/1.6.1 (2016-04-27) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5072 Lines: 164 And now with a proper changelog, and the accidentall dropped call to flush_tlb_kernel_range reinstated: --- >From f720cc324498ab5e7931c7ccb1653bd9b8cddc63 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 15 Oct 2016 18:39:44 +0200 Subject: mm: rewrite __purge_vmap_area_lazy Remove the purge lock, there was nothing left to be protected: - purge_fragmented_blocks seems to has it's own local protection. - all handling of of valist is implicity protected by the atomic list deletion in llist_del_all, which also avoids multiple callers stomping on each other here. - the manipulation of vmap_lazy_nr already is atomic - flush_tlb_kernel_range does not require any synchronization - the calls to __free_vmap_area are sychronized by vmap_area_lock - *start and *end always point to on-stack variables, never mind that the caller never looks at the updated values anyway. Once that is done we can remove the sync argument by moving the calls to purge_fragmented_blocks_allcpus into the callers that need it, and the forced flush_tlb_kernel_range call even if no entries were found into the one caller that cares, and we can also pass start and end by reference. Signed-off-by: Christoph Hellwig --- mm/vmalloc.c | 84 +++++++++++++++++++----------------------------------------- 1 file changed, 26 insertions(+), 58 deletions(-) diff --git a/mm/vmalloc.c b/mm/vmalloc.c index f2481cb..c3ca992 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -613,82 +613,44 @@ void set_iounmap_nonlazy(void) atomic_set(&vmap_lazy_nr, lazy_max_pages()+1); } -/* - * Purges all lazily-freed vmap areas. - * - * If sync is 0 then don't purge if there is already a purge in progress. - * If force_flush is 1, then flush kernel TLBs between *start and *end even - * if we found no lazy vmap areas to unmap (callers can use this to optimise - * their own TLB flushing). - * Returns with *start = min(*start, lowest purged address) - * *end = max(*end, highest purged address) - */ -static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end, - int sync, int force_flush) +static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end) { - static DEFINE_SPINLOCK(purge_lock); struct llist_node *valist; struct vmap_area *va; struct vmap_area *n_va; int nr = 0; - /* - * If sync is 0 but force_flush is 1, we'll go sync anyway but callers - * 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(); - valist = llist_del_all(&vmap_purge_list); llist_for_each_entry(va, valist, purge_list) { - if (va->va_start < *start) - *start = va->va_start; - if (va->va_end > *end) - *end = va->va_end; + if (va->va_start < start) + start = va->va_start; + if (va->va_end > end) + end = va->va_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); - - if (nr) { - spin_lock(&vmap_area_lock); - llist_for_each_entry_safe(va, n_va, valist, purge_list) - __free_vmap_area(va); - spin_unlock(&vmap_area_lock); - } - spin_unlock(&purge_lock); -} + if (!nr) + return false; -/* - * Kick off a purge of the outstanding lazy areas. Don't bother if somebody - * is already purging. - */ -static void try_purge_vmap_area_lazy(void) -{ - unsigned long start = ULONG_MAX, end = 0; + atomic_sub(nr, &vmap_lazy_nr); + flush_tlb_kernel_range(start, end); - __purge_vmap_area_lazy(&start, &end, 0, 0); + spin_lock(&vmap_area_lock); + llist_for_each_entry_safe(va, n_va, valist, purge_list) + __free_vmap_area(va); + spin_unlock(&vmap_area_lock); + return true; } /* - * Kick off a purge of the outstanding lazy areas. + * Kick off a purge of the outstanding lazy areas, including the fragment + * blocks on the per-cpu lists. */ static void purge_vmap_area_lazy(void) { - unsigned long start = ULONG_MAX, end = 0; + purge_fragmented_blocks_allcpus(); + __purge_vmap_area_lazy(ULONG_MAX, 0); - __purge_vmap_area_lazy(&start, &end, 1, 0); } /* @@ -706,8 +668,12 @@ static void free_vmap_area_noflush(struct vmap_area *va) /* After this point, we may free va at any time */ llist_add(&va->purge_list, &vmap_purge_list); + /* + * Kick off a purge of the outstanding lazy areas. Don't bother to + * to purge the per-cpu lists of fragmented blocks. + */ if (unlikely(nr_lazy > lazy_max_pages())) - try_purge_vmap_area_lazy(); + __purge_vmap_area_lazy(ULONG_MAX, 0); } /* @@ -1094,7 +1060,9 @@ void vm_unmap_aliases(void) rcu_read_unlock(); } - __purge_vmap_area_lazy(&start, &end, 1, flush); + purge_fragmented_blocks_allcpus(); + if (!__purge_vmap_area_lazy(start, end) && flush) + flush_tlb_kernel_range(start, end); } EXPORT_SYMBOL_GPL(vm_unmap_aliases); -- 2.1.4