[PATCH 1/6] drm/ttm: Add unampping of the entire device address space
Thomas Hellström (Intel)
thomas_os at shipmail.org
Wed Jun 10 14:07:48 UTC 2020
On 6/10/20 3:54 PM, Andrey Grodzovsky wrote:
>
>
> On 6/10/20 6:15 AM, Thomas Hellström (Intel) wrote:
>>
>>
>> On 6/9/20 7:21 PM, Koenig, Christian wrote:
>>>
>>>
>>> Am 09.06.2020 18:37 schrieb "Grodzovsky, Andrey"
>>> <Andrey.Grodzovsky at 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 at 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.
>>
>
> Can you explain more on this - specifically, which function to reserve
> the BO, why BO reservation would prevent re-fault of the PTE ?
>
The bo reservation is taken in the TTM fault handler and temporarily
blocks inserting a new PTE. So typically the bo reservation is held
around unmap_mapping_range() and the buffer object operation that
triggered it (typically a move or change of backing store).
/Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200610/96660ae9/attachment.htm>
More information about the amd-gfx
mailing list