On 6/9/20 7:21 PM, Koenig, Christian wrote:


Am 09.06.2020 18:37 schrieb "Grodzovsky, Andrey" <andrey.grodzov...@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.grodzov...@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



_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to