From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: ioctl to query a bo's cache level Date: Thu, 22 Sep 2011 16:49:12 +0100 Message-ID: References: <1316632769-2587-1-git-send-email-daniel.vetter@ffwll.ch> <1316656031-11239-1-git-send-email-ben@bwidawsk.net> <1316656031-11239-2-git-send-email-ben@bwidawsk.net> <20110922073512.GC2924@phenom.ffwll.local> <20110922153650.GA10332@cloud01> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 855CA9E912 for ; Thu, 22 Sep 2011 08:51:23 -0700 (PDT) In-Reply-To: <20110922153650.GA10332@cloud01> 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: Ben Widawsky , Daniel Vetter Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 22 Sep 2011 15:36:51 +0000, Ben Widawsky wrote: > On Thu, Sep 22, 2011 at 09:35:12AM +0200, Daniel Vetter wrote: > > On Wed, Sep 21, 2011 at 06:47:10PM -0700, Ben Widawsky wrote: > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > index a546a71..362da16 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > @@ -3777,6 +3777,29 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) > > > } > > > > > > int > > > +i915_gem_get_cache_type_ioctl(struct drm_device *dev, void *data, > > > + struct drm_file *file_priv) > > > +{ > > > + > > > + struct drm_i915_gem_get_cache_type *args = data; > > > + struct drm_i915_gem_object *obj; > > > + int ret = 0; > > > + > > > + obj = to_intel_bo(drm_gem_object_lookup(dev, file_priv, args->handle)); > > > + if (&obj->base == NULL) { > > > + ret = -ENOENT; > > > + goto out; > > > + } > > > + > > > + args->cache_level = obj->cache_level; > > > > Grab struct_mutex around this, obj->cache_level might change over the > > lifetime of the bo. Yeah, our locking is a mess, I know ;-) > > I don't see how a lock by itself helps. The cache_level could change as soon as > you release the lock which results in the same problem. Yes, you will always have the issue of coordination between processes, but you do need the look around the unref, or use drm_gem_object_unreference_unlocked(&obj->base); -Chris -- Chris Wilson, Intel Open Source Technology Centre