Re: [PATCH 2/2] drm/amdgpu: make an improvement on amdgpu_hmm_range_get_pages

[Felix Kuehling <felix.kuehling@amd.com>]

On 2023-12-11 05:38, Christian König wrote:
> Am 09.12.23 um 00:01 schrieb James Zhu:
>> Needn't do schedule for each hmm_range_fault, and use cond_resched
>> to replace schedule.
>
> cond_resched() is usually NAKed upstream since it is a NO-OP in most 
> situations.

That's weird, because https://docs.kernel.org/RCU/stallwarn.html 
specifically recommends it to resolve RCU stall warnings. I previously 
told James to use that instead of schedule().

Regards,
   Felix


>
> IIRC there was even a patch set to completely remove it.
>
> Christian.
>
>>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>> index b24eb5821fd1..c77c4eceea46 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>> @@ -199,6 +199,7 @@ int amdgpu_hmm_range_get_pages(struct 
>> mmu_interval_notifier *notifier,
>>           hmm_range->notifier_seq = mmu_interval_read_begin(notifier);
>>           r = hmm_range_fault(hmm_range);
>>           if (unlikely(r)) {
>> +            cond_resched();
>>               /*
>>                * FIXME: This timeout should encompass the retry from
>>                * mmu_interval_read_retry() as well.
>> @@ -212,7 +213,6 @@ int amdgpu_hmm_range_get_pages(struct 
>> mmu_interval_notifier *notifier,
>>               break;
>>           hmm_range->hmm_pfns += MAX_WALK_BYTE >> PAGE_SHIFT;
>>           hmm_range->start = hmm_range->end;
>> -        schedule();
>>       } while (hmm_range->end < end);
>>         hmm_range->start = start;
>