Re: [RFC 1/8] drm/i915: Add a new PMU for handling non-OA counter data profiling requests

From: "Gupta, Sourab" <sourab.gupta@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Wu, Jabin" <jabin.wu@intel.com>,
	"Woo, Insoo" <insoo.woo@intel.com>
Subject: Re: [RFC 1/8] drm/i915: Add a new PMU for handling non-OA counter data profiling requests
Date: Wed, 5 Aug 2015 09:40:25 +0000	[thread overview]
Message-ID: <1438767767.13551.35.camel@sourabgu-desktop> (raw)
In-Reply-To: <20150805092223.GE10387@nuc-i3427.alporthouse.com>

On Wed, 2015-08-05 at 09:22 +0000, Chris Wilson wrote:
> On Wed, Aug 05, 2015 at 11:25:37AM +0530, sourab.gupta@intel.com wrote:
> > +static void gen_buffer_destroy(struct drm_i915_private *i915)
> > +{
> > +	mutex_lock(&i915->dev->struct_mutex);
> > +	vunmap(i915->gen_pmu.buffer.addr);
> > +	i915_gem_object_ggtt_unpin(i915->gen_pmu.buffer.obj);
> > +	drm_gem_object_unreference(&i915->gen_pmu.buffer.obj->base);
> > +	mutex_unlock(&i915->dev->struct_mutex);
> > +
> > +	spin_lock(&i915->gen_pmu.lock);
> > +	i915->gen_pmu.buffer.obj = NULL;
> > +	i915->gen_pmu.buffer.gtt_offset = 0;
> > +	i915->gen_pmu.buffer.addr = NULL;
> > +	spin_unlock(&i915->gen_pmu.lock);
> 
> This ordering looks scary. At the very least it deserves a comment to
> explain why it is safe.
> 
> So what stops a second event being created while the first is being
> destroyed? I presume the perf events are exclusive? Or a refcounted
> singleton?
> -Chris
> 
Hi Chris,

Yes, the perf events are exclusive. This patch doesn't handle the
problem of exclusion fully. I intended to handle this problem in the
later patch in the series: 
http://lists.freedesktop.org/archives/intel-gfx/2015-August/072959.html
If you check here, a new event init checks whether obj is NULL (while
holding the spinlock), to see whether it is exclusive.
This is taken care of during the event destroy work fn, which assigns
obj to NULL (while holding spinlock), after it is done with everything.

Regards,
Sourab
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx