Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756031AbZICSCu (ORCPT ); Thu, 3 Sep 2009 14:02:50 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754857AbZICSCt (ORCPT ); Thu, 3 Sep 2009 14:02:49 -0400 Received: from gw1.cosmosbay.com ([212.99.114.194]:42343 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753063AbZICSCs (ORCPT ); Thu, 3 Sep 2009 14:02:48 -0400 Message-ID: <4AA00400.1030005@gmail.com> Date: Thu, 03 Sep 2009 19:59:28 +0200 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.23 (Windows/20090812) MIME-Version: 1.0 To: Christoph Lameter CC: Pekka Enberg , Zdenek Kabelac , Patrick McHardy , Robin Holt , Linux Kernel Mailing List , Jesper Dangaard Brouer , Linux Netdev List , Netfilter Developers , paulmck@linux.vnet.ibm.com Subject: Re: [PATCH] slub: fix slab_pad_check() References: <4A87CE60.4020506@gmail.com> <4A896324.3040104@trash.net> <4A9EEF07.5070800@gmail.com> <4A9F1620.2080105@gmail.com> <84144f020909022331x2b275aa5n428f88670e0ae8bc@mail.gmail.com> <4A9F7283.1090306@gmail.com> <4A9FCDC6.3060003@gmail.com> <4A9FDA72.8060001@gmail.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Thu, 03 Sep 2009 19:59:28 +0200 (CEST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3466 Lines: 79 Christoph Lameter a ?crit : > On Thu, 3 Sep 2009, Eric Dumazet wrote: > >> Point is we cannot deal with RCU quietness before disposing the slab cache, >> (if SLAB_DESTROY_BY_RCU was set on the cache) since this disposing *will* >> make call_rcu() calls when a full slab is freed/purged. > > There is no need to do call_rcu calls for frees at that point since > objects are no longer in use. We could simply disable SLAB_DESTROY_BY_RCU > for the final clearing of caches. > >> And when RCU grace period is elapsed, the callback *will* need access to >> the cache we want to dismantle. Better to not have kfreed()/poisoned it... > > But going through the RCU period is pointless since no user of the cache > remains. > >> I believe you mix two RCU uses here. >> >> 1) The one we all know, is use normal caches (!SLAB_DESTROY_BY_RCU) >> (or kmalloc()), and use call_rcu(... kfree_something) >> >> In this case, you are 100% right that the subsystem itself has >> to call rcu_barrier() (or respect whatever self-synchro) itself, >> before calling kmem_cache_destroy() >> >> 2) The SLAB_DESTROY_BY_RCU one. >> >> Part of cache dismantle needs to call rcu_barrier() itself. >> Caller doesnt have to use rcu_barrier(). It would be a waste of time, >> as kmem_cache_destroy() will refill rcu wait queues with its own stuff. > > The dismantling does not need RCU since there are no operations on the > objects in progress. So simply switch DESTROY_BY_RCU off for close. > > > --- > mm/slub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: linux-2.6/mm/slub.c > =================================================================== > --- linux-2.6.orig/mm/slub.c 2009-09-03 10:14:51.000000000 -0500 > +++ linux-2.6/mm/slub.c 2009-09-03 10:18:32.000000000 -0500 > @@ -2594,9 +2594,9 @@ static inline int kmem_cache_close(struc > */ > void kmem_cache_destroy(struct kmem_cache *s) > { > - if (s->flags & SLAB_DESTROY_BY_RCU) > - rcu_barrier(); > down_write(&slub_lock); > + /* Stop deferring frees so that we can immediately free structures */ > + s->flags &= ~SLAB_DESTROY_BY_RCU; > s->refcount--; > if (!s->refcount) { > list_del(&s->list); It seems very smart, but needs review of all callers to make sure no slabs are waiting for final freeing in call_rcu queue on some cpu. I suspect most of them will then have to use rcu_barrier() before calling kmem_cache_destroy(), so why not factorizing code in one place ? net/dccp/ipv6.c:1145: .slab_flags = SLAB_DESTROY_BY_RCU, net/dccp/ipv4.c:941: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv4/udp.c:1593: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv4/udplite.c:54: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv4/tcp_ipv4.c:2446: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv4/udp.c.orig:1587: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv6/udp.c:1274: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv6/udplite.c:52: .slab_flags = SLAB_DESTROY_BY_RCU, net/ipv6/tcp_ipv6.c:2085: .slab_flags = SLAB_DESTROY_BY_RCU, net/netfilter/nf_conntrack_core.c:1269: 0, SLAB_DESTROY_BY_RCU, NULL); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/