linux-kernel - Re: [PATCH v19 22/30] drm/shmem-helper: Add common memory shrinker

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 26 Jan 2024 19:27:49 +0300
From: Dmitry Osipenko <dmitry.osipenko@...labora.com>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: David Airlie <airlied@...il.com>, Gerd Hoffmann <kraxel@...hat.com>,
 Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu
 <olvaffe@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 Christian König <christian.koenig@....com>,
 Qiang Yu <yuq825@...il.com>, Steven Price <steven.price@....com>,
 Emma Anholt <emma@...olt.net>, Melissa Wen <mwen@...lia.com>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 kernel@...labora.com, virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v19 22/30] drm/shmem-helper: Add common memory shrinker

On 1/26/24 12:55, Boris Brezillon wrote:
> On Fri, 26 Jan 2024 00:56:47 +0300
> Dmitry Osipenko <dmitry.osipenko@...labora.com> wrote:
> 
>> On 1/25/24 13:19, Boris Brezillon wrote:
>>> On Fri,  5 Jan 2024 21:46:16 +0300
>>> Dmitry Osipenko <dmitry.osipenko@...labora.com> wrote:
>>>   
>>>> +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem)
>>>> +{
>>>> +	return (shmem->madv >= 0) && shmem->base.funcs->evict &&
>>>> +		refcount_read(&shmem->pages_use_count) &&
>>>> +		!refcount_read(&shmem->pages_pin_count) &&
>>>> +		!shmem->base.dma_buf && !shmem->base.import_attach &&
>>>> +		!shmem->evicted;  
>>>
>>> Are we missing
>>>
>>>                 && dma_resv_test_signaled(shmem->base.resv,
>>> 					  DMA_RESV_USAGE_BOOKKEEP)
>>>
>>> to make sure the GPU is done using the BO?
>>> The same applies to drm_gem_shmem_is_purgeable() BTW.
>>>
>>> If you don't want to do this test here, we need a way to let drivers
>>> provide a custom is_{evictable,purgeable}() test.
>>>
>>> I guess we should also expose drm_gem_shmem_shrinker_update_lru_locked()
>>> to let drivers move the GEMs that were used most recently (those
>>> referenced by a GPU job) at the end of the evictable LRU.  
>>
>> We have the signaled-check in the common drm_gem_evict() helper:
>>
>> https://elixir.bootlin.com/linux/v6.8-rc1/source/drivers/gpu/drm/drm_gem.c#L1496
> 
> Ah, indeed. I'll need DMA_RESV_USAGE_BOOKKEEP instead of
> DMA_RESV_USAGE_READ in panthor, but I can add it in the driver specific
> ->evict() hook (though that means calling dma_resv_test_signaled()
> twice, which is not great, oh well).

Maybe we should change drm_gem_evict() to use BOOKKEEP. The
test_signaled(BOOKKEEP) should be a "stronger" check than
test_signaled(READ)?

> The problem about the evictable LRU remains though: we need a way to let
> drivers put their BOs at the end of the list when the BO has been used
> by the GPU, don't we?

If BO is use, then it won't be evicted, while idling BOs will be
evicted. Hence, the used BOs will be naturally moved down the LRU list
each time shrinker is invoked.

-- 
Best regards,
Dmitry