On 07.03.22 15:22, Jarkko Sakkinen wrote: > On Mon, Mar 07, 2022 at 11:12:44AM +0100, David Hildenbrand wrote: >> On 06.03.22 06:32, Jarkko Sakkinen wrote: >>> For device memory (aka VM_IO | VM_PFNMAP) MAP_POPULATE does nothing. Allow >>> to use that for initializing the device memory by providing a new callback >>> f_ops->populate() for the purpose. >>> >>> SGX patches are provided to show the callback in context. >>> >>> An obvious alternative is a ioctl but it is less elegant and requires >>> two syscalls (mmap + ioctl) per memory range, instead of just one >>> (mmap). >> >> What about extending MADV_POPULATE_READ | MADV_POPULATE_WRITE to support >> VM_IO | VM_PFNMAP (as well?) ? > > What would be a proper point to bind that behaviour? For mmap/mprotect it'd > be probably populate_vma_page_range() because that would span both mmap() > and mprotect() (Dave's suggestion in this thread). MADV_POPULATE_* ends up in faultin_vma_page_range(), right next to populate_vma_page_range(). So it might require a similar way to hook into the driver I guess. > > For MAP_POPULATE I did not have hard proof to show that it would be used > by other drivers but for madvice() you can find at least a few ioctl > based implementations: > > $ git grep -e madv --and \( -e ioc \) drivers/ > drivers/gpu/drm/i915/gem/i915_gem_ioctls.h:int i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > drivers/gpu/drm/i915/i915_driver.c: DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_RENDER_ALLOW), > drivers/gpu/drm/i915/i915_gem.c:i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > drivers/gpu/drm/msm/msm_drv.c:static int msm_ioctl_gem_madvise(struct drm_device *dev, void *data, > drivers/gpu/drm/msm/msm_drv.c: DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise, DRM_RENDER_ALLOW), > drivers/gpu/drm/panfrost/panfrost_drv.c:static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > drivers/gpu/drm/vc4/vc4_drv.c: DRM_IOCTL_DEF_DRV(VC4_GEM_MADVISE, vc4_gem_madvise_ioctl, DRM_RENDER_ALLOW), > drivers/gpu/drm/vc4/vc4_drv.h:int vc4_gem_madvise_ioctl(struct drm_device *dev, void *data, > drivers/gpu/drm/vc4/vc4_gem.c:int vc4_gem_madvise_ioctl(struct drm_device *dev, void *data, > > IMHO this also provides supportive claim for MAP_POPULATE, and yeah, I > agree that to be consistent implementation, both madvice() and MAP_POPULATE > should work. MADV_POPULATE_WRITE + MADV_DONTNEED/FALLOC_FL_PUNCH_HOLE is one way to dynamically manage memory consumption inside a sparse memory mapping (preallocate/populate via MADV_POPULATE_WRITE, discard via MADV_DONTNEED/FALLOC_FL_PUNCH_HOLE). Extending that whole mechanism to deal with VM_IO | VM_PFNMAP mappings as well could be interesting. At least I herd about some ideas where we might want to dynamically expose memory to a VM (via virtio-mem) inside a sparse memory mapping, and the memory in that sparse memory mapping is provided from a dedicated memory pool managed by a device driver -- not just using ordinary anonymous/file/hugetlb memory as we do right now. Now, this is certainly stuff for the future, I just wanted to mention it. -- Thanks, David / dhildenb
