Hello Chris,
On Jul 1, 2013 8:53 PM, "Chris Wilson" <chris@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Jul 01, 2013 at 08:14:42PM +0900, Seung-Woo Kim wrote:
> > Hello Chris,
> >
> > On 2013년 07월 01일 19:57, Chris Wilson wrote:
> > > On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
> > >> +
> > >> +out_close:
> > >> + if (dev->driver->postclose)
> > >> + dev->driver->postclose(dev, priv);
> > >> +out_free:
> > >> kfree(priv);
> > >> filp->private_data = NULL;
> > >> return ret;
> > >
> > > Looks like we are also missing:
> > >
> > > if (drm_core_check_feature(dev, DRIVER_PRIME))
> > > drm_prime_destroy_file_private(&file_priv->prime);
> >
> > Currently, file_priv->prime is just initialized, and
> > drm_prime_destroy_file_private() just checks the list is empty and at
> > the open time, prime list is always empty. So IMHO, it seems unnecessary
> > to call drm_prime_destroy_file_private().
> >
> > If this is necessary, drm_gem_release() is also needed because the pair
> > function of drm_gem_open() is drm_gem_release().
>
> Hmm, could be a slight ordering bug in drm_release(), but yes I agree
> that we should also call drm_gem_release() here in drm_open_helper. It
> is better for the code to be symmetric in cleaning up anything that we
> create so that we are correct for future changes. We might as well fix it
> correctly, rather than having to rediscover this bug at some later date.
> -Chris
>
Thank you for quick reviews.
We'll post V3 patch with this also.
Best regards YJ
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
