From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 17/18] drm/i915: Use a slab for object allocation Date: Wed, 7 Nov 2012 13:59:31 +0000 Message-ID: <20121107135931.69055dde@bwidawsk.net> References: <1350666204-8101-1-git-send-email-chris@chris-wilson.co.uk> <1350666204-8101-17-git-send-email-chris@chris-wilson.co.uk> <20121105174935.579306dc@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from shiva.chad-versace.us (209-20-75-48.static.cloud-ips.com [209.20.75.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 1785B9E7B0 for ; Wed, 7 Nov 2012 05:59:06 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, 05 Nov 2012 20:57:05 +0000 Chris Wilson wrote: > On Mon, 5 Nov 2012 17:49:35 +0000, Ben Widawsky wrote: > > On Fri, 19 Oct 2012 18:03:23 +0100 > > Chris Wilson 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 > > > Reviewed-by: Jesse Barnes > > > > 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 Widawsky, Intel Open Source Technology Center