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@xxxxxxxxxxxxxxxxxx> wrote:
On Tue, 08 Feb 2011 08:52:52 +1000, Dave Airlie <airlied@xxxxxxxxxx> 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
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
