On Wed, 10 Apr 2019 15:07:28 +0200, Chris Wilson wrote: > > Quoting Takashi Iwai (2019-04-10 12:03:22) > > On Wed, 10 Apr 2019 12:44:49 +0200, > > Takashi Iwai wrote: > > > > > > On Wed, 10 Apr 2019 12:24:24 +0200, > > > Chris Wilson wrote: > > > > > > > > Quoting Takashi Iwai (2019-04-10 11:09:47) > > > > > On Wed, 10 Apr 2019 10:17:33 +0200, > > > > > Chris Wilson wrote: > > > > > > > > > > > > While we only allow a single display power reference, the current > > > > > > acquisition/release is racy and a direct call may run concurrently with > > > > > > a runtime-pm worker. Prevent the double unreference by atomically > > > > > > tracking the display_power_active cookie. > > > > > > > > > > > > Testcase: igt/i915_pm_rpm/module-reload #glk-dsi > > > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > > > Cc: Takashi Iwai <tiwai@xxxxxxx> > > > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > > > > > > > > I rather prefer a more straightforward conversion, e.g. something like > > > > > below. Checking the returned cookie as the state flag is not quite > > > > > intuitive, so revive the boolean state flag, and handle it > > > > > atomically. > > > > > > > > Access to the cookie itself is not atomic there, and theoretically > > > > there could be a get/put/get running concurrently. Are you sure don't > > > > want a refcount and lock here? :) > > > > > > The refcount is what we want to reduce, so the suitable option would > > > be a (yet another) mutex although the cmpxchg() should be enough for > > > normal cases. > > > > > > > Your call. For the case CI is hitting, it should do the trick (as we are > > > > only seeing the race on put/put I think). CI will answer in a hour or > > > > two. > > > > > > OK, once when it seems working, I'll respin a patch with a mutex > > > instead. We have already a one that is used for the link state > > > change, and this can be reused. > > > > It's even simpler, so maybe this is a better way to go... > > > > If this is confirmed to work, feel free to queue via drm tree. > > I can't apply this because this is on top of your recent cookie and > > sub-component changes that aren't on sound git tree (yet). > > Success \o/ > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Ok, we'll plonk it in dinq, but I think it should apply to sound.git? > Looks fairly separate. Indeed, it's applicable, so I'm going to queue via sound tree. I thought it would conflict but that was about the previous version fiddling with cmpxchg(). This one is simpler, yeah. > Anyway that can all be resolved in a later merge if required. Sure, I guess it must be trivial, if any. Thanks! Takashi _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
