On Tue, Feb 23, 2021 at 4:59 PM Neil Roberts nroberts@igalia.com wrote:
Daniel Vetter daniel@ffwll.ch writes:
drm_gem_shmem_fault() does not seem to check for purged objects at all.
No idea how this works, or if it ever worked, but yeah something is clearly still busted.
Oh of course, the fault handler doesn’t check this. I’ve added a second patch to make it check and posted it as a separate series here:
https://lists.freedesktop.org/archives/dri-devel/2021-February/298170.html
The two patches combined make the IGT test pass.
Definitely a good idae to have an igt. btw to make that faster you can either use the vm_drop_caches file from proc (it's a bit a hammer), or what I recommend: Have a dedicated debugfs file to only drop everything from your shrinker. That's much quicker and controlled. See e.g. ttm_tt_debugfs_shrink from d4bd7776a7ac ("drm/ttm: rework ttm_tt page limit v4") which recently landed in drm-misc-next.
I agree it would be great to have a debugfs option to trigger the purge. I wonder if someone more involved in Panfrost would like to implement this, because I am actually trying to work on VC4 and this is already turning out to be quite a lot of yak shaving :) I’d also like to implement the same debugfs option and IGT test for VC4.
If we push the shrinker setup into the helpers (this means we minimally need an lru, and probably more reasonable locking that shmem helpers uses right now) then we could have one debugfs file for all drivers supporting purgeable objects. Could then even share some of the igt, only the ioctl code would need to be driver specific.
It's a bit of work though. -Daniel
Thanks for the feedback.
Regards,
- Neil