Re: [PATCH] drm/i915/selftests: Fix unlocked mm.stolen in reset selftest

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi Andi,

On Wed Aug 26, 2026 at 7:31 PM CEST, Andi Shyti wrote:
> Hi Sebastian,
>
> Have you checked sashiko's reviews? They look valid to me.
>
I checked, and there are already existing issues, probably wrth fixing,
but they're outside the scope of this patch.

> ...
>
>> @@ -95,9 +96,12 @@ __igt_reset_stolen(struct intel_gt *gt,
>>  				      ggtt->error_capture.start,
>>  				      PAGE_SIZE);
>>  
>> -		if (!__drm_mm_interval_first(&gt->i915->mm.stolen,
>> -					     page << PAGE_SHIFT,
>> -					     ((page + 1) << PAGE_SHIFT) - 1))
>> +		mutex_lock(&gt->i915->mm.stolen_lock);
>> +		unused = !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> +						  page << PAGE_SHIFT,
>> +						  ((page + 1) << PAGE_SHIFT) - 1);
>> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>> +		if (unused)
>
> this is always false.
Nothing really changes here, even if the if statement is always false,
we still need to guarantee safe access.

>
>>  			memset_io(s, STACK_MAGIC, PAGE_SIZE);
>
> And this needs to be inside the lock.
I don't really understand why, could you tell me more please?

>
>>  
>>  		in = (void __force *)s;
>> @@ -143,10 +147,12 @@ __igt_reset_stolen(struct intel_gt *gt,
>>  			in = tmp;
>>  		x = crc32_le(0, in, PAGE_SIZE);
>>  
>> -		if (x != crc[page] &&
>> -		    !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> -					     page << PAGE_SHIFT,
>> -					     ((page + 1) << PAGE_SHIFT) - 1)) {
>> +		mutex_lock(&gt->i915->mm.stolen_lock);
>> +		unused = !__drm_mm_interval_first(&gt->i915->mm.stolen,
>> +						  page << PAGE_SHIFT,
>> +						  ((page + 1) << PAGE_SHIFT) - 1);
>> +		mutex_unlock(&gt->i915->mm.stolen_lock);
>> +		if (x != crc[page] && unused) {
>
> this is also always false and besides it needs to be included
> inside the locks.
Same here.

-- 
Best regards,
Sebastian