[Devel] [PATCH RHEL7 COMMIT] ms/slub: do not assert not having lock in removing freed partial

Konstantin Khorenko khorenko at virtuozzo.com
Wed Mar 16 02:55:53 PDT 2016


The commit is pushed to "branch-rh7-3.10.0-327.10.1.vz7.12.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.10.1.vz7.12.2
------>
commit 872f0b5e711fd0850bd5ea4d3142804b8e383788
Author: Steven Rostedt <rostedt at goodmis.org>
Date:   Wed Mar 16 13:55:53 2016 +0400

    ms/slub: do not assert not having lock in removing freed partial
    
    Vladimir reported the following issue:
    
    Commit c65c1877bd68 ("slub: use lockdep_assert_held") requires
    remove_partial() to be called with n->list_lock held, but free_partial()
    called from kmem_cache_close() on cache destruction does not follow this
    rule, leading to a warning:
    
      WARNING: CPU: 0 PID: 2787 at mm/slub.c:1536 __kmem_cache_shutdown+0x1b2/0x1f0()
      Modules linked in:
      CPU: 0 PID: 2787 Comm: modprobe Tainted: G        W    3.14.0-rc1-mm1+ #1
      Hardware name:
       0000000000000600 ffff88003ae1dde8 ffffffff816d9583 0000000000000600
       0000000000000000 ffff88003ae1de28 ffffffff8107c107 0000000000000000
       ffff880037ab2b00 ffff88007c240d30 ffffea0001ee5280 ffffea0001ee52a0
      Call Trace:
        __kmem_cache_shutdown+0x1b2/0x1f0
        kmem_cache_destroy+0x43/0xf0
        xfs_destroy_zones+0x103/0x110 [xfs]
        exit_xfs_fs+0x38/0x4e4 [xfs]
        SyS_delete_module+0x19a/0x1f0
        system_call_fastpath+0x16/0x1b
    
    His solution was to add a spinlock in order to quiet lockdep.  Although
    there would be no contention to adding the lock, that lock also requires
    disabling of interrupts which will have a larger impact on the system.
    
    Instead of adding a spinlock to a location where it is not needed for
    lockdep, make a __remove_partial() function that does not test if the
    list_lock is held, as no one should have it due to it being freed.
    
    Also added a __add_partial() function that does not do the lock
    validation either, as it is not needed for the creation of the cache.
    
    Signed-off-by: Steven Rostedt <rostedt at goodmis.org>
    Reported-by: Vladimir Davydov <vdavydov at parallels.com>
    Suggested-by: David Rientjes <rientjes at google.com>
    Acked-by: David Rientjes <rientjes at google.com>
    Acked-by: Vladimir Davydov <vdavydov at parallels.com>
    Acked-by: Christoph Lameter <cl at linux.com>
    Cc: Pekka Enberg <penberg at kernel.org>
    Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
    
    ms commit: 1e4dd9461fabfbc780cdfaf103cec790f3a53325
    ("slub: do not assert not having lock in removing freed partial")
    
    [ported from ms, omitted commit 67b6c900dc6d ("mm: slub: work around
    unneeded lockdep warning") as there is no need in it after this patch]
    
    https://jira.sw.ru/browse/PSBM-43010
    
    Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
---
 mm/slub.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 93068da..7a4ad43 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1508,11 +1508,9 @@ static void discard_slab(struct kmem_cache *s, struct page *page)
 /*
  * Management of partially allocated slabs.
  */
-static inline void add_partial(struct kmem_cache_node *n,
-				struct page *page, int tail)
+static inline void
+__add_partial(struct kmem_cache_node *n, struct page *page, int tail)
 {
-	lockdep_assert_held(&n->list_lock);
-
 	n->nr_partial++;
 	if (tail == DEACTIVATE_TO_TAIL)
 		list_add_tail(&page->lru, &n->partial);
@@ -1520,15 +1518,27 @@ static inline void add_partial(struct kmem_cache_node *n,
 		list_add(&page->lru, &n->partial);
 }
 
-static inline void remove_partial(struct kmem_cache_node *n,
-					struct page *page)
+static inline void add_partial(struct kmem_cache_node *n,
+				struct page *page, int tail)
 {
 	lockdep_assert_held(&n->list_lock);
+	__add_partial(n, page, tail);
+}
 
+static inline void
+__remove_partial(struct kmem_cache_node *n, struct page *page)
+{
 	list_del(&page->lru);
 	n->nr_partial--;
 }
 
+static inline void remove_partial(struct kmem_cache_node *n,
+					struct page *page)
+{
+	lockdep_assert_held(&n->list_lock);
+	__remove_partial(n, page);
+}
+
 /*
  * Remove slab from the partial list, freeze it and
  * return the pointer to the freelist.
@@ -2906,7 +2916,12 @@ static void early_kmem_cache_node_alloc(int node)
 	init_kmem_cache_node(n);
 	inc_slabs_node(kmem_cache_node, node, page->objects);
 
-	add_partial(n, page, DEACTIVATE_TO_HEAD);
+	/*
+	 * No locks need to be taken here as it has just been
+	 * initialized and there is no concurrent access.
+	 */
+
+	__add_partial(n, page, DEACTIVATE_TO_HEAD);
 }
 
 static void free_kmem_cache_nodes(struct kmem_cache *s)
@@ -3192,7 +3207,7 @@ static void free_partial(struct kmem_cache *s, struct kmem_cache_node *n)
 
 	list_for_each_entry_safe(page, h, &n->partial, lru) {
 		if (!page->inuse) {
-			remove_partial(n, page);
+			__remove_partial(n, page);
 			discard_slab(s, page);
 		} else {
 			list_slab_objects(s, page,


More information about the Devel mailing list