On Tue, Jan 17, 2023 at 06:52:35PM +0100, Nirmoy Das wrote: > Currently there is no easy way for a drm driver to safely check and allow > drm_vma_offset_node for a drm file just once. Allow drm drivers to call > non-refcounted version of drm_vma_node_allow() so that a driver doesn't > need to keep track of each drm_vma_node_allow() to call subsequent > drm_vma_node_revoke() to prevent memory leak. > > Cc: Maarten Lankhorst <[email protected]> > Cc: Maxime Ripard <[email protected]> > Cc: Thomas Zimmermann <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: Tvrtko Ursulin <[email protected]> > Cc: Andi Shyti <[email protected]> >
Next time, please, don't leave any spaces between tags. > Suggested-by: Chris Wilson <[email protected]> > Signed-off-by: Nirmoy Das <[email protected]> Reviewed-by: Andi Shyti <[email protected]> Thanks, Andi > --- > drivers/gpu/drm/drm_vma_manager.c | 76 ++++++++++++++++++++++--------- > include/drm/drm_vma_manager.h | 1 + > 2 files changed, 55 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vma_manager.c > b/drivers/gpu/drm/drm_vma_manager.c > index 7de37f8c68fd..83229a031af0 100644 > --- a/drivers/gpu/drm/drm_vma_manager.c > +++ b/drivers/gpu/drm/drm_vma_manager.c > @@ -240,27 +240,8 @@ void drm_vma_offset_remove(struct drm_vma_offset_manager > *mgr, > } > EXPORT_SYMBOL(drm_vma_offset_remove); > > -/** > - * drm_vma_node_allow - Add open-file to list of allowed users > - * @node: Node to modify > - * @tag: Tag of file to remove > - * > - * Add @tag to the list of allowed open-files for this node. If @tag is > - * already on this list, the ref-count is incremented. > - * > - * The list of allowed-users is preserved across drm_vma_offset_add() and > - * drm_vma_offset_remove() calls. You may even call it if the node is > currently > - * not added to any offset-manager. > - * > - * You must remove all open-files the same number of times as you added them > - * before destroying the node. Otherwise, you will leak memory. > - * > - * This is locked against concurrent access internally. > - * > - * RETURNS: > - * 0 on success, negative error code on internal failure (out-of-mem) > - */ > -int drm_vma_node_allow(struct drm_vma_offset_node *node, struct drm_file > *tag) > +static int vma_node_allow(struct drm_vma_offset_node *node, > + struct drm_file *tag, bool ref_counted) > { > struct rb_node **iter; > struct rb_node *parent = NULL; > @@ -282,7 +263,8 @@ int drm_vma_node_allow(struct drm_vma_offset_node *node, > struct drm_file *tag) > entry = rb_entry(*iter, struct drm_vma_offset_file, vm_rb); > > if (tag == entry->vm_tag) { > - entry->vm_count++; > + if (ref_counted) > + entry->vm_count++; > goto unlock; > } else if (tag > entry->vm_tag) { > iter = &(*iter)->rb_right; > @@ -307,8 +289,58 @@ int drm_vma_node_allow(struct drm_vma_offset_node *node, > struct drm_file *tag) > kfree(new); > return ret; > } > + > +/** > + * drm_vma_node_allow - Add open-file to list of allowed users > + * @node: Node to modify > + * @tag: Tag of file to remove > + * > + * Add @tag to the list of allowed open-files for this node. If @tag is > + * already on this list, the ref-count is incremented. > + * > + * The list of allowed-users is preserved across drm_vma_offset_add() and > + * drm_vma_offset_remove() calls. You may even call it if the node is > currently > + * not added to any offset-manager. > + * > + * You must remove all open-files the same number of times as you added them > + * before destroying the node. Otherwise, you will leak memory. > + * > + * This is locked against concurrent access internally. > + * > + * RETURNS: > + * 0 on success, negative error code on internal failure (out-of-mem) > + */ > +int drm_vma_node_allow(struct drm_vma_offset_node *node, struct drm_file > *tag) > +{ > + return vma_node_allow(node, tag, true); > +} > EXPORT_SYMBOL(drm_vma_node_allow); > > +/** > + * drm_vma_node_allow_once - Add open-file to list of allowed users > + * @node: Node to modify > + * @tag: Tag of file to remove > + * > + * Add @tag to the list of allowed open-files for this node. > + * > + * The list of allowed-users is preserved across drm_vma_offset_add() and > + * drm_vma_offset_remove() calls. You may even call it if the node is > currently > + * not added to any offset-manager. > + * > + * This is not ref-counted unlike drm_vma_node_allow() hence > drm_vma_node_revoke() > + * should only be called once after this. > + * > + * This is locked against concurrent access internally. > + * > + * RETURNS: > + * 0 on success, negative error code on internal failure (out-of-mem) > + */ > +int drm_vma_node_allow_once(struct drm_vma_offset_node *node, struct > drm_file *tag) > +{ > + return vma_node_allow(node, tag, false); > +} > +EXPORT_SYMBOL(drm_vma_node_allow_once); > + > /** > * drm_vma_node_revoke - Remove open-file from list of allowed users > * @node: Node to modify > diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h > index 4f8c35206f7c..6c2a2f21dbf0 100644 > --- a/include/drm/drm_vma_manager.h > +++ b/include/drm/drm_vma_manager.h > @@ -74,6 +74,7 @@ void drm_vma_offset_remove(struct drm_vma_offset_manager > *mgr, > struct drm_vma_offset_node *node); > > int drm_vma_node_allow(struct drm_vma_offset_node *node, struct drm_file > *tag); > +int drm_vma_node_allow_once(struct drm_vma_offset_node *node, struct > drm_file *tag); > void drm_vma_node_revoke(struct drm_vma_offset_node *node, > struct drm_file *tag); > bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node, > -- > 2.39.0
