On 25/07/2019 22:28, Rob Herring wrote:
On Thu, Jul 25, 2019 at 9:35 AM Steven Price <steven.price@xxxxxxx> wrote:
On 25/07/2019 15:59, Steven Price wrote:
[...]
It would appear that in the following call sgt==NULL:
ret = sg_alloc_table_from_pages(sgt, pages + page_offset,
NUM_FAULT_PAGES, 0, SZ_2M, GFP_KERNEL);
Which means we've ended up with a BO with bo->sgt==NULL, bo->pages set
and bo->is_heap=true. My understanding is this should be impossible.
I haven't yet figured out how this happens - it seems to be just before
termination, so it might be a race with cleanup?
That was a red herring - it's partly my test case doing something a bit
weird. This crash is caused by doing an mmap of a HEAP object before any
fault has occurred.
My brother Red is always causing problems. ;)
But you were right that I need to kfree bo->sgts. Additionally, I also
need to call sg_free_table on each table.
drm_gem_shmem_mmap() calls drm_gem_shmem_get_pages() which will populate
bo->base.pages (even if bo->is_heap).
Either we should prevent mapping of HEAP objects, or alternatively
bo->base.pages could be allocated upfront instead of during the first
fault. My preference would be allocating it upfront because optimising
for the case of a HEAP BO which isn't used seems a bit weird. Although
there's still the question of exactly what the behaviour should be of
accessing through the CPU pages which haven't been allocated yet.
Also shmem->pages_use_count needs incrementing to stop
drm_gem_shmem_get_pages() replacing bo->base.pages. I haven't tested
what happens if you mmap *after* the first fault.
I did say mmap had undefined/unknown behavior.
True - and I was surprised to find out my test was actually doing that!
But crashing the kernel is perhaps a bit excessive!
Avoiding the mmap of HEAP objects everything runs fine - and the memory
leaks are much smaller than they used to be :)
Steve
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
