| Message ID | 20230315105446.5858-1-nirmoy.das@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: Clarify seemingly unaccounted obj refcount inc | expand |
Hi Nirmoy, On Wed, Mar 15, 2023 at 11:54:46AM +0100, Nirmoy Das wrote: > Add a comment why there is a obj refcount inc before installing > the vm_ops for the mmap call. Also remove the invalid older comment > as drm API(drm_gem_prime_mmap()) will hold an obj reference before > calling this driver mmap callback so we can't have 0-refcnted > object here. > > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> fine with me! Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi > --- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index d3c1dee16af2..0bc8c3818443 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -952,9 +952,10 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) > vma_pages(vma)); > if (node && drm_vma_node_is_allowed(node, priv)) { > /* > - * Skip 0-refcnted objects as it is in the process of being > - * destroyed and will be invalid when the vma manager lock > - * is released. > + * When we install vm_ops for mmap we are too late for > + * the vm_ops->open() which increases the ref_count of > + * this obj and then it gets decreased by the vm_ops->close(). > + * To balance this increase the obj ref_count here. > */ > if (!node->driver_private) { > mmo = container_of(node, struct i915_mmap_offset, vma_node); > -- > 2.39.0
On 3/15/2023 11:54 AM, Nirmoy Das wrote: > Add a comment why there is a obj refcount inc before installing > the vm_ops for the mmap call. Also remove the invalid older comment > as drm API(drm_gem_prime_mmap()) will hold an obj reference before > calling this driver mmap callback so we can't have 0-refcnted > object here. > > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index d3c1dee16af2..0bc8c3818443 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -952,9 +952,10 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) > vma_pages(vma)); > if (node && drm_vma_node_is_allowed(node, priv)) { > /* > - * Skip 0-refcnted objects as it is in the process of being > - * destroyed and will be invalid when the vma manager lock > - * is released. This valid. Matt pointed out a case when user close the obj and call mmap and driver only have the fake offset to refer the object and can end up calling this while driver is freeing the obj. I will resend with keeping the valid comment. Nirmoy > + * When we install vm_ops for mmap we are too late for > + * the vm_ops->open() which increases the ref_count of > + * this obj and then it gets decreased by the vm_ops->close(). > + * To balance this increase the obj ref_count here. > */ > if (!node->driver_private) { > mmo = container_of(node, struct i915_mmap_offset, vma_node);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index d3c1dee16af2..0bc8c3818443 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -952,9 +952,10 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) vma_pages(vma)); if (node && drm_vma_node_is_allowed(node, priv)) { /* - * Skip 0-refcnted objects as it is in the process of being - * destroyed and will be invalid when the vma manager lock - * is released. + * When we install vm_ops for mmap we are too late for + * the vm_ops->open() which increases the ref_count of + * this obj and then it gets decreased by the vm_ops->close(). + * To balance this increase the obj ref_count here. */ if (!node->driver_private) { mmo = container_of(node, struct i915_mmap_offset, vma_node);
Add a comment why there is a obj refcount inc before installing the vm_ops for the mmap call. Also remove the invalid older comment as drm API(drm_gem_prime_mmap()) will hold an obj reference before calling this driver mmap callback so we can't have 0-refcnted object here. Cc: Matthew Auld <matthew.auld@intel.com> Cc: Andi Shyti <andi.shyti@linux.intel.com> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)