[Intel-xe] [PATCH v2 07/31] drm/xe: Only try to lock external BOs in VM bind

[Thomas Hellström]

On 5/8/23 23:34, Rodrigo Vivi wrote:
> On Mon, May 08, 2023 at 01:08:10AM +0000, Matthew Brost wrote:
>> On Fri, May 05, 2023 at 02:40:40PM -0400, Rodrigo Vivi wrote:
>>> On Mon, May 01, 2023 at 05:17:03PM -0700, Matthew Brost wrote:
>>>> Not needed and causes some issues with bulk LRU moves.
>>> I'm confused with this explanation and the code below.
>>> could you please provide a bit more wording here?
>>>
>> We only need to try to lock a BO if it external as non-external BOs
>> share the dma-resv with the already locked VM. Trying to lock
>> non-external BOs caused an issue (list corruption) in an uncoming patch

s/uncoming/upcoming/

Also it's not clear to me how this could fix a list corruption in the 
bulk LRU moves? I mean, if it's a duplicate lock then it gets removed 
from the tv list and not touched again? Could you explain the mechanism 
of the fix?

Thanks,

Thomas


>> which adds bulk LRU move. Since this code isn't needed, remove it.
> it makes more sense now. with this in commit msg (but with Christopher fix)
>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
>
>> ^^^ How about this.
>>
>>>> Signed-off-by: Matthew Brost <mattthew.brost at intel.com>
>>>> ---
>>>>   drivers/gpu/drm/xe/xe_vm.c | 8 +++++---
>>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>>>> index 272f0f7f24fe..6c427ff92c44 100644
>>>> --- a/drivers/gpu/drm/xe/xe_vm.c
>>>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>>>> @@ -2064,9 +2064,11 @@ static int vm_bind_ioctl(struct xe_vm *vm, struct xe_vma *vma,
>>>>   		 */
>>>>   		xe_bo_get(vbo);
>>>>   
>>>> -		tv_bo.bo = &vbo->ttm;
>>>> -		tv_bo.num_shared = 1;
>>>> -		list_add(&tv_bo.head, &objs);
>>>> +		if (!vbo->vm) {
>>>> +			tv_bo.bo = &vbo->ttm;
>>>> +			tv_bo.num_shared = 1;
>>>> +			list_add(&tv_bo.head, &objs);
>>>> +		}
>>>>   	}
>>>>   
>>>>   again:
>>>> -- 
>>>> 2.34.1
>>>>