[PATCH] drm/amdgpu: Fix minmax error

[Christian König]

Am 25.11.22 um 08:56 schrieb Luben Tuikov:
> On 2022-11-25 02:45, Christian König wrote:
>>
>> Am 24.11.22 um 22:19 schrieb Luben Tuikov:
>>> Fix minmax compilation error by using min_t()/max_t(), of the assignment type.
>>>
>>> Cc: James Zhu <James.Zhu at amd.com>
>>> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
>>> Fixes: 58170a7a002ad6 ("drm/amdgpu: fix stall on CPU when allocate large system memory")
>>> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 10 +++++++---
>>>    1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> index 8a2e5716d8dba2..d22d14b0ef0c84 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> @@ -191,14 +191,18 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>>>    	hmm_range->dev_private_owner = owner;
>>>    
>>>    	do {
>>> -		hmm_range->end = min(hmm_range->start + MAX_WALK_BYTE, end);
>>> +		hmm_range->end = min_t(typeof(hmm_range->end),
>>> +				       hmm_range->start + MAX_WALK_BYTE,
>>> +				       end);
>> Since end is a local variable I would strongly prefer to just have it
>> use the correct type for it.
>>
>> Otherwise we might end up using something which doesn't work on all
>> architectures.
> They all appear to be "unsigned long". I thought, since we assign to
> hmm_range->end, we use that type.

Mhm, then why does the compiler complain here?

As far as I can see "unsigned long" is correct here, but if we somehow 
have a typecast then something is not working as expected.

Is MAX_WALK_BYTE maybe of signed type?

>
> Would you prefer at the top of the function to define "timeout" and "end" as,
> 	typeof(hmm_range->end) end, timeout;

Well for end that might make sense, but timeout is independent of the 
hmm range.

Regards,
Christian.

>
> Regards,
> Luben
>