perfect. the whole structure's much much clearer now thanks to just that comment. atleast that did it for me
maybe move the "Typically we are called (via the driver) from drm_stub_open()" to the function annotation aswell but not strictly needed with the new comment On Tue, Feb 8, 2011 at 1:18 AM, Chris Wilson <chris at chris-wilson.co.uk>wrote: > On Tue, 08 Feb 2011 08:52:52 +1000, Dave Airlie <airlied at redhat.com> > wrote: > > On Sun, 2011-02-06 at 19:24 +0200, adam zeira wrote: > > > is this a problem? before trying to submit a solution I wanted to make > sure with the experts > > > > > > it seems open_count isn't locked and can cause problems > > > > > > and if it is a problem, and needs to be solved, should locking be done > or is it better implemented with the (as I understand standard) kref? > > > > > > > Ickle? > > > > 1a72d65d6291ec248cbc5f05df2487edd714aba6 was your doing and I'm not > > entirely sure you actually checked all the paths looking back. > > Would this clarify matters? Perhaps there is some spare annotation that > could be used here as well? > > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c > index 2ec7d48..5b4ca5b 100644 > --- a/drivers/gpu/drm/drm_fops.c > +++ b/drivers/gpu/drm/drm_fops.c > @@ -125,6 +125,13 @@ int drm_open(struct inode *inode, struct file *filp) > struct drm_minor *minor; > int retcode = 0; > > + /* Typically we are called (via the driver) from drm_stub_open() > + * as their own fops->open and so already hold the global mutex. > + * Enforce this. > + */ > + if (WARN_ON(!mutex_is_locked(&drm_global_mutex))) > + return -EINVAL; > + > minor = idr_find(&drm_minors_idr, minor_id); > if (!minor) > return -ENODEV; > > -- > Chris Wilson, Intel Open Source Technology Centre > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20110209/3a27322d/attachment.html>
