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. 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. Steve _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
