Re: [PATCH] drm/ttm: fix ttm_bo_unreserve

["Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>]

On 2019-06-05 1:24 p.m., Christian König wrote:
> Am 04.06.19 um 21:03 schrieb Zeng, Oak:
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Kuehling, Felix
>> On 2019-06-04 11:23, Christian König wrote:
>>
>>> Since we now keep BOs on the LRU we need to make sure that they are
>>> removed when they are pinned.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>    include/drm/ttm/ttm_bo_driver.h | 14 ++++++--------
>>>    1 file changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/drm/ttm/ttm_bo_driver.h
>>> b/include/drm/ttm/ttm_bo_driver.h index 9f54cf9c60df..c9b8ba492f24
>>> 100644
>>> --- a/include/drm/ttm/ttm_bo_driver.h
>>> +++ b/include/drm/ttm/ttm_bo_driver.h
>>> @@ -767,14 +767,12 @@ static inline int
>>> ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo,
>>>     */
>>>    static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
>>>    {
>>> -    if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
>>> -        spin_lock(&bo->bdev->glob->lru_lock);
>>> -        if (list_empty(&bo->lru))
>>> -            ttm_bo_add_to_lru(bo);
>>> -        else
>>> -            ttm_bo_move_to_lru_tail(bo, NULL);
>>> -        spin_unlock(&bo->bdev->glob->lru_lock);
>>> -    }
>>> +    spin_lock(&bo->bdev->glob->lru_lock);
>>> +    if (list_empty(&bo->lru))
>>> +        ttm_bo_add_to_lru(bo);
>>> +    else
>>> +        ttm_bo_move_to_lru_tail(bo, NULL);
>> Going just by the function names, this seems to do the exact opposite
>> of what the change description says.
>>
>> [Oak] +1, when I read the description, I also get lost...So please do
>> add a more accurate description.
> 
> I'm puzzled why you are confused. We now keep the BOs on the LRU while
> they are reserved, so on unreserve we now need to explicitly remove them
> from the LRU when they are pinned.

I don't know about Felix and Oak, but for me "remove from the LRU" is
confusing, as I don't see that in the code, only adding to the LRU or
moving to its tail.


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx