On Wed, Jan 16, 2019 at 11:40:41AM +0000, Emil Velikov wrote:
> On 2019/01/14, Daniel Vetter wrote:
> > On Mon, Jan 14, 2019 at 08:44:10AM +0000, Emil Velikov wrote:
> > > From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> > >
> > > Currently we fail to free and detach the drm_file when drm_setup() fails.
> > > Use the drm_close_helper to do address that.
> > >
> > > Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/drm_file.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > index 149506a20bdc..871dcd8c7545 100644
> > > --- a/drivers/gpu/drm/drm_file.c
> > > +++ b/drivers/gpu/drm/drm_file.c
> > > @@ -330,8 +330,10 @@ int drm_open(struct inode *inode, struct file *filp)
> > > goto err_undo;
> > > if (need_setup) {
> > > retcode = drm_setup(dev);
> > > - if (retcode)
> > > + if (retcode) {
> > > + drm_close_helper(filp);
> >
> > I freaked out mildly because drm_open_helper already adds the drm_file to
> > the filelist (hence publishes it), and publishing objects before they're
> > fully set up is a Bad Idea :-)
> >
> > But drm_setup only does legacy setup, so who cares. On both patches:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >
> Thanks.
>
> > And if you feel like doing an s/drm_setup()/drm_legacy_setup()/, with or
> > w/o changing the condition to if (need_setup &&
> > drm_core_check_feature(dev, DRIVER_LEGACY)), then that patch would also
> > have my
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >
> I'll leave that for another day, since the inverse vfunc (firstopen <>
> lastclose) is not a legacy only one. Which kind of irks me a bit.
We need lastclose for the fbdev emulation. Which is already fixed through
the new drm_client stuff and the new generic fbdev. It's a matter of
rolling that out. The other use is for delayed vgaswitcheroo changes,
which I think is a bit a hack ... Since I don't have a solution for that I
guess lasclose will stay with us for a while longer.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
