On Mon, 05 Nov 2012 20:57:05 +0000 Chris Wilson <chris at chris-wilson.co.uk> wrote: > On Mon, 5 Nov 2012 17:49:35 +0000, Ben Widawsky <ben at bwidawsk.net> wrote: > > On Fri, 19 Oct 2012 18:03:23 +0100 > > Chris Wilson <chris at chris-wilson.co.uk> wrote: > > > > > The primary purpose of this was to debug some use-after-free memory > > > corruption that was causing an OOPS inside drm/i915. As it turned out > > > the corruption was being caused elsewhere and i915.ko as a major user of > > > many objects was being hit hardest. > > > > > > Indeed as we do frequent the generic kmalloc caches, dedicating one to > > > ourselves (or at least naming one for us depending upon the core) aids > > > debugging our own slab usage. > > > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > > > Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org> > > > > So I previously acked this patch, but you dropped that, so now you get > > more words. > > > > Why not go all the way and move all allocations we do in i915 to this > > slab (adding a flag with whether or not to zero the memory first)? While > > I agree in terms of space, nothing takes up as much as objects, I think > > it would be nice to do this so we can totally track what our driver is > > doing. In fact I would suggest this is something which core drm should > > provide to all of the drivers (we already use drm_alloc calls in some > > places). > > I thought the slab cache was a fixed size, so presumably you mean > introduce a slab per object we allocate? Of which the only other > interesting one is the requests. And we don't tend to have many requests > pending at any one time (outside of exceptional error cases), unlike the > thousands of bo that tend to be allocated. So is it worth dedicating a > separate slab to the few but frequently allocated requests? > -Chris > I guess I wasn't really thinking about the details. I knew in the back of my mind that the slab cache was a fixed size, but was thinking we could have a generically large enough object for most of our other things (I think they're mostly small except for dev_priv). But, now thinking about it a bit, I guess that wastes the memory we're trying to scrutinize, so it's probably silly. In any case, this made me review kmem_cache_create a bit, so we can bump this patch to: Reviewed-by: Ben Widawsky <ben at bwidawsk.net> -- Ben Widawsky, Intel Open Source Technology Center
