Re: [PATCH 3/5] drm/i915: Convert OpRegion ASLE irq worker into a tasklet

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 23 May 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, May 23, 2016 at 12:12:30PM +0300, Jani Nikula wrote:
>> >  #define ACPI_EV_DISPLAY_SWITCH (1<<0)
>> > @@ -814,11 +807,11 @@ void intel_opregion_fini(struct drm_device *dev)
>> >  	if (!opregion->header)
>> >  		return;
>> >  
>> > +	tasklet_kill(&dev_priv->opregion.asle_task);
>> > +
>> 
>> So what if you got a new asle interrupt right here?
>
> Before we call fini, we should have de-installed the irq and done
> synchronize_irq, so we only have to worry about the residual task.
> (At least that is what I expect!)

I'd expect that too, but looks like

i915_driver_unload -> i915_driver_unregister -> intel_opregion_fini

happens *before*, not after

i915_driver_unload -> intel_modeset_cleanup -> intel_irq_uninstall

J.


>
>> >  	if (opregion->asle)
>> >  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
>> 
>> This is supposed to signal we're not ready to handle said interrupts
>> anymore. Not that we should rely on it either.
>> 
>> It wasn't pretty before, but I think this patch widens the window for a
>> race. If you kept the *other* code as it were, and just changed the work
>> to tasklets, I'd be willing to look in the other direction...
>
> Considering the recent discussion about the negatives of
> tasklets/ksoftirqd, I think I was being too cavalier in this conversion,
> and we should only think about using tasklet where the post-interrupt
> latency is critical.
> -Chris

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx