On Thu, 4 Jan 2007, Pekka Enberg wrote: > Hi Hugh, > > [Sorry, no access to kernel tree right now, so can't send a patch.] > > On 1/4/07, Hugh Dickins <[EMAIL PROTECTED]> wrote: > > @@ -3310,7 +3310,7 @@ retry: > > */ > > goto retry; > > } else { > > - kmem_freepages(cache, obj); > > + /* cache_grow already freed obj */ > > obj = NULL; > > So, how about we rename the current cache_grow() to __cache_grow() and > move the kmem_freepages() to a higher level function like this: > > static int cache_grow(struct kmem_cache *cache, > gfp_t flags, int nodeid) > { > void *objp; > int ret; > > if (flags & __GFP_NO_GROW) > return 0; > > objp = kmem_getpages(cachep, flags, nodeid); > if (!objp) > return 0; > > ret = __cache_grow(cache, flags, nodeid, objp); > if (!ret) > kmem_freepages(cachep, objp); > > return ret; > } > > And use the non-allocating __cache_grow version() in fallback_alloc() instead?
That does indeed look more tasteful. But there appears to be a fair bit more refactoring one would want to do, if aiming for good taste there: the kmem_flagcheck, the conditional local_irq_dis/enable... I think I'll leave that to you and Christoph to fight over later! Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/