<div dir="ltr"><div>perfect. the whole structure's much much clearer now thanks to just that comment. atleast that did it for me</div><div><br></div><div>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</div>
<div><br></div><div><div><div class="gmail_quote">On Tue, Feb 8, 2011 at 1:18 AM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
On Tue, 08 Feb 2011 08:52:52 +1000, Dave Airlie <<a href="mailto:airlied@redhat.com">airlied@redhat.com</a>> wrote:<br>
> On Sun, 2011-02-06 at 19:24 +0200, adam zeira wrote:<br>
> > is this a problem? before trying to submit a solution I wanted to make sure with the experts<br>
> ><br>
> > it seems open_count isn't locked and can cause problems<br>
> ><br>
> > 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?<br>
> ><br>
><br>
> Ickle?<br>
><br>
> 1a72d65d6291ec248cbc5f05df2487edd714aba6 was your doing and I'm not<br>
> entirely sure you actually checked all the paths looking back.<br>
<br>
Would this clarify matters? Perhaps there is some spare annotation that<br>
could be used here as well?<br>
<br>
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c<br>
index 2ec7d48..5b4ca5b 100644<br>
--- a/drivers/gpu/drm/drm_fops.c<br>
+++ b/drivers/gpu/drm/drm_fops.c<br>
@@ -125,6 +125,13 @@ int drm_open(struct inode *inode, struct file *filp)<br>
struct drm_minor *minor;<br>
int retcode = 0;<br>
<br>
+ /* Typically we are called (via the driver) from drm_stub_open()<br>
+ * as their own fops->open and so already hold the global mutex.<br>
+ * Enforce this.<br>
+ */<br>
+ if (WARN_ON(!mutex_is_locked(&drm_global_mutex)))<br>
+ return -EINVAL;<br>
+<br>
minor = idr_find(&drm_minors_idr, minor_id);<br>
if (!minor)<br>
return -ENODEV;<br>
<font color="#888888"><br>
--<br>
Chris Wilson, Intel Open Source Technology Centre<br>
</font></blockquote></div><br></div></div></div>