Re: [PATCH v2 1/3] drm/i915: Improve readability of eb_relocate_vma

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

Hi Krzysztof
On Thu Jul 17, 2025 at 2:38 PM UTC, Krzysztof Karas wrote:
> Hi Sebastian,
>
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -1529,6 +1529,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>>  	struct drm_i915_gem_relocation_entry __user *urelocs =
>>  		u64_to_user_ptr(entry->relocs_ptr);
>>  	unsigned long remain = entry->relocation_count;
>> +	int ret = 0;
>>  
>>  	if (unlikely(remain > N_RELOC(INT_MAX)))
>>  		return -EINVAL;
>> @@ -1541,11 +1542,10 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
>>  	if (unlikely(!access_ok(urelocs, remain * sizeof(*urelocs))))
>>  		return -EFAULT;
>>  
>> -	do {
>> -		struct drm_i915_gem_relocation_entry *r = stack;
>> -		unsigned int count =
>> -			min_t(unsigned long, remain, ARRAY_SIZE(stack));
>> +	while (remain > 0) {
> Is it possible for "remain" variable to be initialized to 0?
> If that would be the case then after your change we'd skip this
> loop entirely, where previously we'd run at least one iteration.
> Would that be a problem?
-- 
This should not occur, as there are several direct and indirect safeguards
verifying that entry->relocation_count is non-zero.
e.g.
```eb_relocate_vma_slow
1622 ›       for (i = 0; i < entry->relocation_count; i++) {
```
or

```check_relocations
1642 ›       size = entry->relocation_count;
1643 ›       if (size == 0)
1644 ›       ›       return 0;
```
And it would be a peculiar choice to copy 0 bytes

Best regards,
Sebastian