On 6/26/19 10:58 AM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2019-06-26 18:38:45)
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.
Yes. Looking at the second patch, that is clearer wrt what data we are
guarding with the locks.
Don't drop the intel_ prefix from intel_uncore_debug as it looks to
still be visible outside of its enclave (it's getting long, but it should
I'm assuming you're referring to the 2 new functions. These are actually
meant to only used within the file and not be externally visible, I just
forgot the static tag (and sparse complained for that). Everything else
should still have the intel_ prefix.
Daniele
be more or less internal to the various intel_uncore implementations) and
squash these 2 as I feel more comfortable with intel_uncore_debug taking
control of the locking as required for sharing and reviewing the locking
wrt to the shared data.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
