drm/ttm: new TT backend allocation pool

[Christian König]

Am 26.10.20 um 06:23 schrieb Dave Airlie:
> On Mon, 26 Oct 2020 at 01:41, Christian König
> <ckoenig.leichtzumerken at gmail.com> wrote:
>> This replaces the spaghetti code in the two existing page pools.
>>
>> First of all depending on the allocation size it is between 3 (1GiB) and
>> 5 (1MiB) times faster than the old implementation.
>>
>> It makes better use of buddy pages to allow for larger physical contiguous
>> allocations which should result in better TLB utilization at least for amdgpu.
>>
>> Instead of a completely braindead approach of filling the pool with one CPU
>> while another one is trying to shrink it we only give back freed pages.
>>
>> This also results in much less locking contention and a trylock free MM
>> shrinker callback, so we can guarantee that pages are given back to the system
>> when needed.
>>
>> Downside of this is that it takes longer for many small allocations until the
>> pool is filled up. We could address this, but I couldn't find an use case
>> where this actually matters. And we don't bother freeing large chunks of pages
>> any more.
>>
>> The sysfs files are replaced with a single module parameter, allowing users to
>> override how many pages should be globally pooled in TTM. This unfortunately
>> breaks the UAPI slightly, but as far as we know nobody ever depended on this.
>>
>> Zeroing memory coming from the pool was handled inconsistently. The
>> alloc_pages() based pool was zeroing it, the dma_alloc_attr() based one wasn't.
>> The new implementation isn't zeroing pages from the pool either and only sets
>> the __GFP_ZERO flag when necessary.
>>
>> The implementation has only 753 lines of code compared to the over 2600 of the
>> old one, and also allows for saving quite a bunch of code in the drivers since
>> we don't need specialized handling there any more based on kernel config.
>>
>> Additional to all of that there was a neat bug with IOMMU, coherent DMA
>> mappings and huge pages which is now fixed in the new code as well.
>>
>> Please review and comment,
> Interesting, 5 doesn't have appeared to make on the list, but it
> definitely has some checkpatch warnings. (indents, missing spaces
> etc), Please clean those up.

Well #5 is the really interesting one. Going to address the checkpatch 
warnings and send that out again with probably the first patch already 
pushed.

>
> some other random comments on it
>
> +       if (order) {
> +               gfp_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY |
> +                       __GFP_KSWAPD_RECLAIM;
> +               gfp_flags &= ~__GFP_MOVABLE;
> +               gfp_flags &= ~__GFP_COMP;
> +       }
>
> I'd like some explains of why these flags are chosen.

Good question, I don't remember fully either and have just copied them 
over from the old allocator for the hugepage case.

In general we just want a cheap try to allocate and fallback to 4k if we 
don't have enough free.

Moveable and and compound don't work with how TTMs fault mechanism 
works, so that makes sense anyway.

> Otherwise I'm pretty happy with the remaining patch in the series, it
> ends up with a pretty nice cleanup.
>
> Note drm_gem_vram_helper fails to build (vmm->dev should be dev->dev possibly).

Ok, going to double check that. What I've also noticed is that QXL 
doesn't seem to have a device structure at all.

Going to support that in the new allocator as well.

> Once you clean up checkpatch and make drm_gem_vram_helper build again.
> For 5-13 Reviewed-by: Dave Airlie <airlied at redhat.com>
>
> I've also boot tested this on nouveau and it survives the basics fine.

Nice to know. I've tested quite a bit on amdgpu and smoke tested on 
radeon/nouveau.

Going to give it a try on my AGP radeon as well when I have time.

Christian.

>
> Dave.
>
>> Christian.
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel