10 Jun
2020
10 Jun
'20
10:15 a.m.
On 6/9/20 7:21 PM, Koenig, Christian wrote:
Am 09.06.2020 18:37 schrieb "Grodzovsky, Andrey" Andrey.Grodzovsky@amd.com:
On 6/5/20 2:40 PM, Christian König wrote: > Am 05.06.20 um 16:29 schrieb Andrey Grodzovsky: >> >> On 5/11/20 2:45 AM, Christian König wrote: >>> Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: >>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> >>>> --- >>>> drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- >>>> include/drm/ttm/ttm_bo_driver.h | 2 ++ >>>> 2 files changed, 23 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c >>>> b/drivers/gpu/drm/ttm/ttm_bo.c >>>> index c5b516f..eae61cc 100644 >>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>>> @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct >>>> ttm_buffer_object *bo) >>>> ttm_bo_unmap_virtual_locked(bo); >>>> ttm_mem_io_unlock(man); >>>> } >>>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual); >>>> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) >>>> +{ >>>> + struct ttm_mem_type_manager *man; >>>> + int i; >>>> -EXPORT_SYMBOL(ttm_bo_unmap_virtual); >>> >>>> + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { >>>> + man = &bdev->man[i]; >>>> + if (man->has_type && man->use_type) >>>> + ttm_mem_io_lock(man, false); >>>> + } >>> >>> You should drop that it will just result in a deadlock warning for >>> Nouveau and has no effect at all. >>> >>> Apart from that looks good to me, >>> Christian. >> >> >> As I am considering to re-include this in V2 of the patchsets, can >> you clarify please why this will have no effect at all ? > > The locks are exclusive for Nouveau to allocate/free the io address > space. > > Since we don't do this here we don't need the locks. > > Christian. So basically calling unmap_mapping_range doesn't require any extra locking around it and whatever locks are taken within the function should be enough ?I think so, yes.
Christian.
Yes, that's true. However, without the bo reservation, nothing stops a PTE from being immediately re-faulted back again. Even while unmap_mapping_range() is running. So the device removed flag needs to be advertized before this function is run, (perhaps with a memory barrier pair). That should probably be added to the function documentation.
(Other than that, please add a commit message if respinning).
/Thomas