[Outreachy kernel] [PATCH] drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()

[Julia Lawall]

On Tue, 26 Sep 2017, Daniel Vetter wrote:

> On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
> > The IDR deletion interface now returns the deleted entry or NULL if it was not
> > present. So we don't have to do the extra work of checking if we have a
> > reference on the drm_gem_object, this can be handled by checking the return
> > value from idr_remove() and the extra locks can be dropped.
> >
> > Signed-off-by: Aishwarya Pant <aishpant at gmail.com>
>
> Haneen is already working on this task, with the patch just merged. Please
> coordinate with mentors (me or Sean Paul) before starting on something to
> avoid such clashes in the future.
>
> Note also that some todo items are just ideas, and need confirmation with
> relevant maintainers first.

Not sure where the mixup happened, but anyone who starts on a project
specific task should note that on the outreachy kernel wiki and before
starting on a project specific task one should check that no one is
working on it.  If someone started some time ago, and the code doesn't
seem to have changed, ask the person who claimed the task or the mentor.

thanks,

julia

>
> Thanks, Daniel
> > ---
> >  drivers/gpu/drm/drm_gem.c | 21 ++-------------------
> >  1 file changed, 2 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index c55f338..f62757a 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
> >  {
> >  	struct drm_gem_object *obj;
> >
> > -	/* This is gross. The idr system doesn't let us try a delete and
> > -	 * return an error code.  It just spews if you fail at deleting.
> > -	 * So, we have to grab a lock around finding the object and then
> > -	 * doing the delete on it and dropping the refcount, or the user
> > -	 * could race us to double-decrement the refcount and cause a
> > -	 * use-after-free later.  Given the frequency of our handle lookups,
> > -	 * we may want to use ida for number allocation and a hash table
> > -	 * for the pointers, anyway.
> > -	 */
> >  	spin_lock(&filp->table_lock);
> > -
> > -	/* Check if we currently have a reference on the object */
> > -	obj = idr_replace(&filp->object_idr, NULL, handle);
> > -	spin_unlock(&filp->table_lock);
> > -	if (IS_ERR_OR_NULL(obj))
> > +	obj = idr_remove(&filp->object_idr, handle);
> > +	if (!obj)
> >  		return -EINVAL;
> > -
> >  	/* Release driver's reference and decrement refcount. */
> >  	drm_gem_object_release_handle(handle, obj, filp);
> > -
> > -	/* And finally make the handle available for future allocations. */
> > -	spin_lock(&filp->table_lock);
> > -	idr_remove(&filp->object_idr, handle);
> >  	spin_unlock(&filp->table_lock);
> >
> >  	return 0;
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe at googlegroups.com.
> > To post to this group, send email to outreachy-kernel at googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40aishwarya.
> > For more options, visit https://groups.google.com/d/optout.
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe at googlegroups.com.
> To post to this group, send email to outreachy-kernel at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170926082047.eqsd2lslpqp4pi5f%40phenom.ffwll.local.
> For more options, visit https://groups.google.com/d/optout.
>