Re: [PATCH 1/2] slub: fix leakage

[Christoph Lameter]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/