On 6/26/19 3:02 AM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)@@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore, void intel_uncore_forcewake_user_get(struct intel_uncore *uncore) { spin_lock_irq(&uncore->lock); + spin_lock(&uncore->debug->lock); if (!uncore->user_forcewake.count++) {Afaict, uncore->user_forcewake.count is only guarded by uncore->lock and we only need to take debug->lock for the debug->unclaimed_mmio_check manipulation. But there needs to be a shared usage counter around the debug as it is shared state.intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);/* Save and disable mmio debugging for the user bypass */uncore->user_forcewake.saved_mmio_check = - uncore->unclaimed_mmio_check; + uncore->debug->unclaimed_mmio_check; uncore->user_forcewake.saved_mmio_debug = i915_modparams.mmio_debug;Something more like spin_lock_irq(&uncore->lock); if (!uncore->user_forcewake_count++) { spin_lock(&uncore->debug->lock); if (!uncore->debug->usage_count++) { ... } spin_unlock(&uncore->debug->lock); } ... spin_unlock_irq(&uncore->lock); ? -Chris
I do something similar in the next patch in the series (with the lock still on the outside of the if). I've split that out because I've added some functional changes to the flow. I can squash the 2 patches if you thing it is better that way.
Daniele _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
