Re: [Intel-gfx] [PATCH] drm/i915: Refine mmio debug flow to avoid bad unlock balance detected.

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

On Tue, 11 Jul 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Hi Lee,
>
> I'm really sorry for the late reply, I had some holidays in
> between.
>
> On Tue, Jul 04, 2023 at 04:07:27PM +0800, Lee Shawn C wrote:
>> Perform reboot stresss test on a kernel build with lockdebug flag enabled.
>> Bad unlock balanace detected would happened sometimes. Below is the
>> problematic scenario. If params.mmio_debug value was changed at step #4.
>> And it would trigger this issue. Modify code flow that decide to
>> enable/disable mmio debug before unclaimed_reg_debug() can avoid
>> this symptom.
>> 
>>     1. GEN6_READ_HEADER is called with params.mmio_debug = 0
>>     2. unclaimed_reg_debug(before = true) is called
>>     3. unclaimed_reg_debug return without taking a lock
>>        because params.mmio_debug == 0
>>     4. other thread modifies params.mmio_debug to 1
>>     5. GEN6_READ_FOOTER is called with params.mmio_debug != 0
>>     6. unclaimed_reg_debug tries to assert non-taken lock (first WARN)
>>     7. unclaimed_reg_debug tries to release non-taken lock (second WARN)
>> 
>> Closes:https://gitlab.freedesktop.org/drm/intel/-/issues/8749
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_uncore.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 796ebfe6c550..9d665978cc43 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1931,9 +1931,6 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
>>  		    const bool read,
>>  		    const bool before)
>>  {
>> -	if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
>> -		return;
>> -
>
> this is a very good catch! I'm fine with the change from my side:
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Nope, there's a bug in the patch.

https://lore.kernel.org/r/87ilb00zot.fsf@intel.com

>
> But I'd like to hear from someone else, as well, Matt, Daniele?
>
> Thank you,
> Andi

-- 
Jani Nikula, Intel Open Source Graphics Center