[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.64.0711051036250.13139@schroedinger.engr.sgi.com>
Date: Mon, 5 Nov 2007 10:45:40 -0800 (PST)
From: Christoph Lameter <clameter@....com>
To: Hugh Dickins <hugh@...itas.com>
cc: Olivér Pintér <oliver.pntr@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Willy Tarreau <w@....eu>, linux-kernel@...r.kernel.org,
stable@...nel.org
Subject: Re: [PATCH 1/2] slub: fix leakage
On Sun, 4 Nov 2007, Hugh Dickins wrote:
> That testing went fine, as you'd expect. Your diffstat is certainly
> nicer than mine (corrected for SlabDebug) would be. I expect you'll
> go ahead with yours.
Thanks for testing.
> But I remain slightly uneasy about it: I do think your original
> instinct, in putting in the code you're now removing, was good.
Well yes. I tend to think too much about performance.
> In a low memory situation, when several tasks pile up to allocate
> the same resource, we'd usually free back all but the first, rather
> than depleting free memory even more than necessary. That you were
> doing before, now you take the simpler way out and don't bother.
Hmmm... But even without the patch: All tasks had to allocate their
own slabs via the page allocator first. Most of those were then thrown
away immediately. Now we are flushing the current cpu slab. Which means
that this is also going back to the page allocator if its empty. It is
likely that the push back in the situation you mention will put a slab
with only one object allocated onto the partial lists. This can have two
beneficial effects:
1. We can avoid going back to the page allocator for awhile since we will
find the almost free slab if the current slab is exhausted.
2. If the object that was allocated in the flushed slab was a short lived
use freed then the slab will go back to the page allocator very fast.
> I've no evidence that this is a significant issue: just mention
> it in case it gives you second thoughts e.g. was there a concrete
> scenario, other than instinct, which led you to put in that code
> originally?
The intend was to use objects that were cache hot as much as possible. Use
of the newly allocated slab means we are likely accessing a cache cold
page.
However, given that it took us pretty long to find that issue I would
think that this is not that of an important code path. So the removal
seems to be the right way to go.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists