Message ID | 20180326090831.22686-5-lionel.g.landwerlin@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 26 March 2018 at 10:08, Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote: > If 2 processes race to open the perf stream, it's possible that one of them > will see that OA buffer has already been allocated, while a previous process > is still finishing to reprogram the hardware (on gen8+). But we grab the perf.lock while opening the perf stream, and we only allow one exclusive stream. Maybe I'm missing something?
On 26/03/18 13:00, Matthew Auld wrote: > On 26 March 2018 at 10:08, Lionel Landwerlin > <lionel.g.landwerlin@intel.com> wrote: >> If 2 processes race to open the perf stream, it's possible that one of them >> will see that OA buffer has already been allocated, while a previous process >> is still finishing to reprogram the hardware (on gen8+). > But we grab the perf.lock while opening the perf stream, and we only > allow one exclusive stream. Maybe I'm missing something? > Arg, you're right... The locking situation is really not obvious in this file. I think we can drop this patch. Thanks!
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index f15dda286cfe..86c18ba46816 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1435,13 +1435,15 @@ static int alloc_oa_buffer(struct drm_i915_private *dev_priv) struct i915_vma *vma; int ret; - if (WARN_ON(dev_priv->perf.oa.oa_buffer.vma)) - return -ENODEV; - ret = i915_mutex_lock_interruptible(&dev_priv->drm); if (ret) return ret; + if (dev_priv->perf.oa.oa_buffer.vma) { + ret = -EBUSY; + goto unlock; + } + BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE); BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);
If 2 processes race to open the perf stream, it's possible that one of them will see that OA buffer has already been allocated, while a previous process is still finishing to reprogram the hardware (on gen8+). The opening sequence has been reworked a few times and we probably lost track of the order in which things are supposed to happen. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_perf.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)