On Mon, 26 Oct 2020 at 01:41, Christian König <ckoenig.leichtzumerken@xxxxxxxxx> 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. 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. 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). Once you clean up checkpatch and make drm_gem_vram_helper build again. For 5-13 Reviewed-by: Dave Airlie <airlied@xxxxxxxxxx> I've also boot tested this on nouveau and it survives the basics fine. Dave. > Christian. > > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel