Re: [PATCH] drm/amdgpu: Fix minmax error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Fixes: 58170a7a002ad6 ("drm/amdgpu: fix stall on CPU when allocate large system memory")
Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>
---
   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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux