Hi Tvrtko,
On 1/18/2023 10:19 AM, Tvrtko Ursulin wrote:
Hi,
Thanks for working on this, it looks good to me and it aligns with how
i915 uses the facility.
Copying Mirsad who reported the issue in case he is still happy to
give it a quick test. Mirsad, I don't know if you are subscribed to
one of the two mailing lists where series was posted. In case not, you
can grab both patches from
https://patchwork.freedesktop.org/series/112952/.
Nirmoy - we also have an IGT written by Chuansheng -
https://patchwork.freedesktop.org/patch/515720/?series=101035&rev=4. A
more generic one could be placed in gem_mmap_offset test but this one
works too in my testing and is IMO better than nothing.
This looks good to me. let's get this merge and I can look into
improving it at later point.
Finally, let me add some tags below:
On 17/01/2023 17:52, Nirmoy Das wrote:
drm_vma_node_allow() and drm_vma_node_revoke() should be called in
balanced pairs. We call drm_vma_node_allow() once per-file everytime a
user calls mmap_offset, but only call drm_vma_node_revoke once per-file
on each mmap_offset. As the mmap_offset is reused by the client, the
per-file vm_count may remain non-zero and the rbtree leaked.
Call drm_vma_node_allow_once() instead to prevent that memory leak.
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Cc: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Fixes: 786555987207 ("drm/i915/gem: Store mmap_offsets in an rbtree
rather than a plain list")
Reported-by: Chuansheng Liu <chuansheng.liu@xxxxxxxxx>
Reported-by: Mirsad Todorovac <mirsad.todorovac@xxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # v5.7+
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Thanks for your review and those extra tags.
Nirmoy
Regards,
Tvrtko
Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 4f69bff63068..2aac6bf78740 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -697,7 +697,7 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
GEM_BUG_ON(lookup_mmo(obj, mmap_type) != mmo);
out:
if (file)
- drm_vma_node_allow(&mmo->vma_node, file);
+ drm_vma_node_allow_once(&mmo->vma_node, file);
return mmo;
err:
